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 627701 - option to encrypt new partitions (using LUKS)
option to encrypt new partitions (using LUKS)
Status: RESOLVED OBSOLETE
Product: gparted
Classification: Other
Component: application
0.5.1
Other Linux
: Normal enhancement
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2010-08-23 09:50 UTC by Liv
Modified: 2020-11-13 10:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add initial luks support (12.77 KB, patch)
2012-09-01 07:55 UTC, Matthias Gehre
none Details | Review
LUKS patchset v1 (84.96 KB, patch)
2012-09-01 22:11 UTC, Matthias Gehre
none Details | Review
LUKS patchset v2 (83.04 KB, patch)
2012-09-03 09:11 UTC, Matthias Gehre
none Details | Review
LUKS patchset v3 (85.20 KB, patch)
2012-09-04 21:10 UTC, Matthias Gehre
none Details | Review
LUKS patchset v4 (85.20 KB, patch)
2012-09-04 21:59 UTC, Matthias Gehre
none Details | Review
LUKS patchset v4 (dlsym version) (87.57 KB, patch)
2012-09-05 08:44 UTC, Matthias Gehre
none Details | Review
LUKS patchset v5 (using dmsetup) (85.90 KB, patch)
2012-09-06 08:57 UTC, Matthias Gehre
none Details | Review
LUKS patchset v6 (93.74 KB, patch)
2012-09-07 08:28 UTC, Matthias Gehre
none Details | Review
LUKS patchset v6.1 (93.76 KB, patch)
2012-09-07 08:36 UTC, Matthias Gehre
none Details | Review
LUKS patchset v7 (124.78 KB, patch)
2012-09-07 12:00 UTC, Matthias Gehre
none Details | Review
LUKS patchset v8 (125.04 KB, patch)
2012-10-10 08:56 UTC, Matthias Gehre
none Details | Review
LUKS patchset v9 (132.19 KB, patch)
2012-10-22 11:46 UTC, Matthias Gehre
none Details | Review
LUKS patchset v10 (88.34 KB, patch)
2012-11-26 04:23 UTC, Matthias Gehre
none Details | Review
GParted encrypted partitions screen shot (67.69 KB, image/png)
2013-01-07 20:52 UTC, Curtis Gedak
  Details
Screenshot showing same setting (53.27 KB, image/png)
2013-02-11 00:11 UTC, Matthias Gehre
  Details
LUKS patchset v11 (88.37 KB, patch)
2013-02-11 00:12 UTC, Matthias Gehre
none Details | Review

Description Liv 2010-08-23 09:50:56 UTC
It would be nice if GParted provided an option to encrypt newly created partitions, perhaps using LUKS as outlined in this article [1]. In plus of the usual operations when creating a new partition, Gparted would need to run something following the lines of: 
dd if=/dev/urandom of=/dev/[your device] bs=4K
cryptsetup luksFormat /dev/[your device] -c aes -s 256 -h sha256
cryptsetup luksOpen /dev/[your device] [device_label]

Next the fs would be created on /dev/mapper/[device_label]. 

Please let me know what you think


[1] http://ubuntu-tutorials.com/2007/08/17/7-steps-to-an-encrypted-partition-local-or-removable-disk/
Comment 1 Matthias Gehre 2012-09-01 07:55:50 UTC
Created attachment 223126 [details] [review]
Add initial luks support

This patch adds some initial support for LUKS.
1. It shows the correct size of the LUKS "fs" (If a mapping is active, it is read from libcryptsetup. If no mapping is active, LUKS spans the whole underlaying partition. There is no size written to disk.)
2. It shows the name of the mapping under "mountpoint" if one is active

I plan to add more LUKS support. For example
1. Handling the LUKS partition like an Extended Partition and showing the contained filesystem below it.
2. Resizing LUKS along with the contained filesystem
Comment 2 Matthias Gehre 2012-09-01 12:56:29 UTC
I did a bit more work on LUKS.
Now gparted shows the contained file system below the LUKS partition in the TreeView. (Just like extended partitions work)

Unfortunately, the patchset got a bit bigger, because I had to do some refactoring in order to reuse the partition detection.

To my feeling, this refactoring cleaned up the base code, but comments are very appreciated, of course!

You can find the code under
https://github.com/mgehre/gparted
in the master branch.

All commits are self-contained (they only depend on the preceeding commits)
and should be small enough to review test.
Comment 3 Curtis Gedak 2012-09-01 19:47:20 UTC
Hi Matthias,

Thank you for taking the initiative to jump in to the GParted code and start developing support for LUKS.  I also appreciate that you posted the patch in this bug report for review.

From working with Mike Fleetwood on Bug #670171 - Add LVM PV read-write support, we have found it helpful to post complete patch sets, as opposed to one by one patches.  This makes review easier, especially if the previously reviewed patches are not changed.

By tracking everything in a single bug report, it is also easier for others to review, comment, and make suggestions for improvement.

As you might already know, if needed to edit a previous commit you can use:

    git rebase bbc643cd^ --interactive

Hence problems with previous commits can be fixed too.  :-)

When reposting a patch set containing previous commit edits, we appreciate a note saying which commits were edited.

To create a complete patch set, you can use a command similar to the following:

    git format-patch origin/master


Now if I recall, you had asked a question in the forum so I will repost it here and respond.

   QUESTION:

   I'm looking forward to your comments.
   Especially, I was not sure about using libcryptsetup vs. calling
   cryptsetup binary.
   I find the first options more elegant, but it seems that the second
   option is used more often throughout the code.


ANSWER:

As you have noticed, both methods are used in GParted.  For example, GParted uses the libparted library, but calls the command directly for blkid from the util-linux package for determining UUID, LABEL, etc.

Both methods have there place.  By calling libraries, there is much less chance for errors in parameter passing, and quite often better error handling is possible.  The disadvantage is that the library must be available at compile time.  This means that if GParted is compiled on a GNU/Linux distribution that does not have the library installed, then a user must not only install the library but must also recompile GParted in order to use the feature supported by the library.

This is not the case if GParted checks for the existence of commands at run time (or the rescan for supported actions button).  In these situations a user simply has to install the required software for GParted to provide support for a specific feature.

If the cryptsetup library is installed by default in most GNU/Linux distributions, then calling the library should be okay.


With comment #2 it appears that you have done some more work on this enhancement.  When you reach a logical point of functionality, please create a single patch set, post it in this report, and I will begin reviewing the code.

I look forward to reviewing your code, and working with you on this enhancement.

Regards,
Curtis Gedak
Comment 4 Curtis Gedak 2012-09-01 19:58:15 UTC
Adding a link back to the feature request in the GParted forum to add LUKS support.  The discussion in the forum contains some valuable information.

http://gparted-forum.surf4.info/viewtopic.php?id=15405
Comment 5 Matthias Gehre 2012-09-01 22:11:34 UTC
Created attachment 223169 [details] [review]
LUKS patchset v1

Implements:

1. Size of LUKS container is shown
2. Name of active mapping of LUKS is shown
3. File system contained in LUKS is shown below LUKS partition in tree view (similar to how extended partitions work)
4. File system contained in LUKS is shown in DrawingAreaVisualDisk
Comment 6 Matthias Gehre 2012-09-02 15:01:22 UTC
Patches 2-7 of this patchset are the same as the patchset from
https://bugzilla.gnome.org/show_bug.cgi?id=683149
Comment 7 Matthias Gehre 2012-09-03 09:11:56 UTC
Created attachment 223262 [details] [review]
LUKS patchset v2

This patchset is the same as v1, but
1. rebased on master (after the lvm2 read-write changes)
2. reordered commits so the first 12 commits are the preparation/cleanup of the gparted code necessary for the last 2 commits adding luks support.
3. squashed commits changing {src/include}/luks.{h,cc} together
Comment 8 Matthias Gehre 2012-09-03 09:19:04 UTC
*** Bug 683149 has been marked as a duplicate of this bug. ***
Comment 9 Curtis Gedak 2012-09-03 23:07:12 UTC
Thank you Matthias for combining the two bug reports into one, and for consolidating the code changes into one patch set.

I plan to review the code soon, though I do need to work this into my day along with several other tasks.  Your patience is appreciated.
Comment 10 Curtis Gedak 2012-09-03 23:44:21 UTC
Hi Matthias,

After applying the patch set from comment #7, I encountered the following error when trying to compile the code:

luks.cc: In static member function ‘static Glib::ustring GParted::luks::find_map_by_device(Glib::ustring, std::vector<Glib::ustring>&)’:
luks.cc:95:49: error: ‘crypt_get_device_name’ was not declared in this scope
luks.cc: In function ‘GParted::Sector GParted::get_partition_size(Glib::ustring, std::vector<Glib::ustring>&)’:
luks.cc:166:2: error: ‘crypt_active_device’ was not declared in this scope
luks.cc:166:22: error: expected ‘;’ before ‘cad’
luks.cc:167:59: error: ‘cad’ was not declared in this scope
luks.cc:167:62: error: ‘crypt_get_active_device’ was not declared in this scope
luks.cc:174:1: warning: control reaches end of non-void function
make[2]: *** [luks.o] Error 1
make[2]: Leaving directory `/home/gedakc/workspace/gparted.dev2/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/gedakc/workspace/gparted.dev2'
make: *** [all] Error 2

My development platform is currently running kubuntu 11.04.  I have the cryptsetup-luks and libcryptsetup-dev packages installed.

Does the patch set from comment #7 compiler on your platform?
Comment 11 Matthias Gehre 2012-09-04 13:43:43 UTC
Hi Curtis,

I'm developing on Ubuntu 12.04, and it seems that libcryptsetup
added exactly those symbols between 11.04 and 12.04.

I can work around this by using the cryptsetup executable instead of libcryptsetup.

To make sure that its output looks the same on your platform,
could you please activate a LUKS mapping by
cryptsetup luksOpen devicePath mappingName
and then print the output of 
cryptsetup status mappingName
?
Comment 12 Curtis Gedak 2012-09-04 16:48:14 UTC
We do try to be backwards compatible for at least the currently supported major GNU/Linux distributions.

Using the cryptsetup executable would have the advantage that it's presence can be determined at runtime.

For the output requested, I created a 23 MiB /dev/sde1 partition initially with the ext2 file system.  Then I set up luks encryption with the following command:

$ sudo cryptsetup luksFormat /dev/sde1 -c aes -s 256 -h sha256

Then I issued the commands requested as follows:

$ sudo cryptsetup luksOpen /dev/sde1 vault
Enter passphrase for /dev/sde1: 
$ sudo cryptsetup status vault
/dev/mapper/vault is active:
  cipher:  aes-cbc-plain
  keysize: 256 bits
  device:  /dev/sde1
  offset:  2112 sectors
  size:    44992 sectors
  mode:    read/write
$ 

Since ubuntu 10.04 is currently supported, I also performed these steps in a VM with ubuntu 10.04.  The results are as follows:

$ sudo cryptsetup luksOpen /dev/sdc1 vault
Enter passphrase for /dev/sdc1: 
device-mapper: remove ioctl failed: Device or resource busy
Key slot 0 unlocked.
$ sudo cryptsetup status vault
/dev/mapper/vault is active:
  cipher:  aes-cbc-plain
  keysize: 256 bits
  device:  /dev/sdc1
  offset:  2056 sectors
  size:    45048 sectors
  mode:    read/write
$ 

From a quick glance it appears that the output format is similar.


If you need to parse out specific text, the GParted method Utils::regexp_label enables you to enter a regular expression to capture the portion of the string you need.  You can search the code for examples of how this has been used.
Comment 13 Matthias Gehre 2012-09-04 21:10:43 UTC
Created attachment 223471 [details] [review]
LUKS patchset v3

Hi,

I added support for libcryptsetup < 1.2.0 by using the old API in that case.

Support for the cryptsetup binary is not in there (yet), but I can add that at a later time. Maybe its better to postpone that to not enlarge the patchset unnecessarily.

I found a problem: If the LUKS device is active, but no filesystem is in the container, then the subtree entry is "unallocated". Using Partition->New
on that space to create a partition/filesystem fails with "/dev/mapper/vault: unrecognized disk label", because the LUKS does not contain a partition table.
Maybe gparted should allow to create a file system without a partition there.

Other operations, such as reformating the an existing file system in the contained space to a different filesystem and delete an existing file system work as expected.
Comment 14 Matthias Gehre 2012-09-04 21:59:15 UTC
Created attachment 223473 [details] [review]
LUKS patchset v4

Fixed a silly compile error in the last patchset.
Comment 15 Curtis Gedak 2012-09-04 22:34:49 UTC
Thank you Matthias for the updated patch set.

I have discovered that the libcryptsetup does not seem to be installed in Ubuntu by default (at least in 11.04), so my preference would be to use the cryptsetup binary sooner rather than later.  That will make it easier for more of our users to run GParted without a recompile.  It also makes it easier for packages since they do not need to add in a hard dependency.

(In reply to comment #13)
> I found a problem: If the LUKS device is active, but no filesystem is in the
> container, then the subtree entry is "unallocated". Using Partition->New
> on that space to create a partition/filesystem fails with "/dev/mapper/vault:
> unrecognized disk label", because the LUKS does not contain a partition table.
> Maybe gparted should allow to create a file system without a partition there.
> 
> Other operations, such as reformating the an existing file system in the
> contained space to a different filesystem and delete an existing file system
> work as expected.

Are you trying to make a partition table inside an luks encrypted partition?

If so then it might be better to focus on something simpler such as support for a luks encrypted partition containing a single file system.  Later we could tackle trying to support a luks encrypted partition containing a partition table.

As you have discovered, GParted supports formatting partitions with file systems, but not formatting an entire device with a file system.  I can see where this might create some challenges for supporting a luks encrypted partition that has only been formatted with a file system.  The challenge with the current GParted design is that LUKS isn't really a file system, which is similar to the situation we faced adding support for LVM Physical Volumes.

Having said that, we were successful in implementing LVM PV similar to file systems in GParted, so with some careful thought we can hopefully do the same with luks.
Comment 16 Matthias Gehre 2012-09-05 08:28:00 UTC
Hi Curtis,

thanks for your input. Would dynamic linking via dlopen() and friends be okay, too? This removes the compile-time dependency, but stills allows to use the nice library interface.

I'm not trying to create a partition table inside a LUKS encrypted partition. I was just trying to format the contained space to some filesystem. But as you said, gparted only allows that while creating a partition.
The problem goes away, if one adds the loop partition table to the LUKS container, as libparted then shows a (virtual) partition, which can be formatted by gparted.

I guess one of the problems is that gparted assumes that all entries in one Treeview belong to the same disk. This is a bit problematic with LUKS, because 
its contained file system has it's own diks (/dev/mapper/something).

On the other hand, the user should not care about those specifics. I feel that it make a lot sense to show the contained file system below the LUKS container (just as this patchset does).

Of course, this would need some more changes to gparted's core. Should I include it into this patchset or move it into a followup patchset?
Comment 17 Matthias Gehre 2012-09-05 08:44:41 UTC
Created attachment 223503 [details] [review]
LUKS patchset v4 (dlsym version)

Please find attached a version of the v4 patchset which uses dynamic binding
via dlsym.
Comment 18 Matthias Gehre 2012-09-05 19:52:46 UTC
Just a note: For the bug in libparted worked around by
luks::fix_partition_path()
I have submitted a patch to the bug-parted mailinglist at
http://lists.gnu.org/archive/html/bug-parted/2012-09/msg00001.html
Comment 19 Matthias Gehre 2012-09-06 08:57:42 UTC
Created attachment 223611 [details] [review]
LUKS patchset v5 (using dmsetup)

Another update :-)
Now its using the dmsetup executable. No depedencies on libcryptsetup or cryptsetup, no dlsym. This also cleaned up the code a bit.

I could not use cryptsetup, because it did not allow to list all active mappings. And the only way to find the mapping name of an device 
(i.e. cryptsetup luksOpen device mapping_name)
is to iterate over all mapping_names and check their backing device.

dmsetup simplifies that a lot, because it allows to output all active mappings with all their necessary information (size, offset, backing device) in on call.


I also fixed 
1. showing the correct mountpoint of the LUKS device instead of just mapping 
name.
2. check for existance of dmsetup binary before setting "fs .read = FS::EXTERNAL"

I also like this version better than the dynamic linking/dynamic binding approaches.
Comment 20 Matthias Gehre 2012-09-06 09:05:19 UTC
Two more notes:

1. The code should also work with loop-aes (actually any dm-crypt)
2. The workaround in patch 11 for "ped_partition_is_busy returns false for busy partitions inside luks containers" is actually a side-effect of libparted using the wrong partition name. This would also be fixed by my patch send upstream.
Comment 21 Curtis Gedak 2012-09-06 17:35:56 UTC
Hi Matthias,

I am glad to see that my delay in responding has not held up your
development efforts.  :-)

The patch set in comment #19 compiles cleanly on my kubuntu 11.04
system, and I have been able to do some preliminary testing.  I was
extremely impressed to see support for resizing file systems within
the encrypted partition.  Great work!

I like your novel approach to displaying the contents of a luksOpen
partition similar to how logical partitions are shown under an
extended partition.  My initial thoughts were that we would need to
treat the luksOpen partition as another device but you have shown me
that there is another approach.

While I have not had sufficient time to review the code closely (and
learn a thing or two from you :-), I have found a few minor
suggestions for improvement.  These are as follows:

1)  Add the include/luks.h to the include/Makefile.am file.

    Doing so will ensure that the necessary source files are included
    in distribution tarballs which are created with "make dist" or
    "make distcheck".  I see that you have already added src/luks.cc
    to src/Makefile.am.

2)  Make strings that are presented to the user translatable into other
    languages.

    Examples of this can be seen in src/GParted)Core.cc by searching
    for the text "_(" without the double quotes.  For situations where
    a parameter is filled in, it is helpful to translators if you
    include an example filled in sentence in a comment immediately
    preceding the line of code containing the string.  Of course
    command strings are exempt from this treatment.

    For strings that might have singular and/or plural substitutions,
    see the ngettext method.  An example can be found in
    src/Win_GParted.cc by searching for ngettext.

3)  Add files containing translatable strings to po/POTFILES.in.

    Doing so ensures that strings within the files, such as
    src/luks.cc, will be added to the list of strings to be
    translated.  Then the translators will be able to add each
    language translation of the string into the respective .po
    language translation files.


Currently I am juggling several different tasks, including writing a
book on GParted, so it may be a while (a few weeks?) before I can
carve out a large enough block of time to give your code the full
review it deserves.
Comment 22 Matthias Gehre 2012-09-07 08:28:05 UTC
Created attachment 223726 [details] [review]
LUKS patchset v6

Hi,

thanks for your comments.
I addressed all points in this updated patchset by changing the "Add initial LUKS support" commit. I did not commit the changes introduced by "make update-po", because I don't know how the policy on that is.

This patchset has two additional commits:
1. luks: change menu text from unmount to deactivate, implement deactivate
    Following the lvm approach, LUKS is not unmounted but deactivated.
    This commit also implements the deactivate operation.

2. do not allow to delete partition on "loop" partition table
    libparted uses the virtual "loop" partition table, if it detects
    a file system on disk but no partition table. (I.e. the disk has
    been directly formatted with a file system). Deleting such a partition
    makes libparted unable to detect the partition table (it does not make
    up the "loop" partition table anymore), and stops it
    from re-adding a partition.
    
    One can still reformat the "partition", or paste into it.
    
    This is at least necessary until gparted
    allows formatting a disk with a file system without a partition table.
    This especially applies to LUKS containers as they usually come without a
    partition table.
Comment 23 Matthias Gehre 2012-09-07 08:36:56 UTC
Created attachment 223727 [details] [review]
LUKS patchset v6.1

Fixed a stupid compiler error in the last patchset.
Comment 24 Matthias Gehre 2012-09-07 12:00:47 UTC
Created attachment 223759 [details] [review]
LUKS patchset v7

Changes:

1. Moved the fix_luks_partition_path() into a new commit "Fix get_partition_path for loop partition tables" and made it more general, as it affects not only LUKS.

2. Added two commits:
2.1 Win_GParted: replace devices[ current_device ] by get_selected_device()
2.2 determine get_selected_device() based on selected_partition
    
    We don't return the device selected in the combo box, but the
    device that the currently selected_partition is on. This may
    be different e.g. when the filesystem is inside a LUKS container.
    
    Previously removing a partition inside a LUKS container would remove the
    partition from the parent device, because GParted was not aware that
    the partition table is on a different device than that selected from the
    combo box.
    
    But this change allows generally for sub-partitions to reside
    on different devices, so it's not at all bound to LUKS. Just think
    about complicated hierarchies possible by dm-mapper, lvm, btrfs
    snapshots, loop devices etc.
    
    This also makes it possible to show multiple devices (like /dev/sda and
    /dev/sdb) together in one tree-view like
    
    /dev/sda
      /dev/sda1 Primary partiton
      /dev/sda2 Extended partition
        /dev/sda3 Logical partition
        /dev/sda4 Logical partition with LUKS
          /dev/mapper/vault1 Primary partition inside LUKS
          /dev/mapper/vault2 Extended partition inside LUKS
            ...
    /dev/sdb
      /dev/sdb1
      ...
    
    (The code for actually putting multiple devices in one TreeView is not
    included. This code only makes sure that each operation targets the
    correct device when multiple devices are shown in one TreeView.)
Comment 25 Curtis Gedak 2012-09-10 16:24:50 UTC
(In reply to comment #22)
> I addressed all points in this updated patchset by changing the "Add initial
> LUKS support" commit. I did not commit the changes introduced by "make
> update-po", because I don't know how the policy on that is.

You guessed right on this on Matthias - the .po files are completely handled by the GNOME Translation Project.  Changes to the .po files are committed when the translators have had a chance to add the appropriate language translations to the .po files.

If you are curious, you can learn more at the GNOME Translation Project web site.
https://live.gnome.org/TranslationProject/
Comment 26 Matthias Gehre 2012-10-07 14:17:27 UTC
Any update?
Comment 27 Curtis Gedak 2012-10-07 16:29:33 UTC
Hi Matthias,

> Any update?

Thanks for asking.  I haven't forgotten about this enhancement, though things probably looks quiet to you.

My current plan is to review your patch set during the week of October 16th.

In addition to normal work activities, I am preparing the GParted 0.14.0 release for Oct 10th so that the release path is clear for new enhancements, such as this one.  On top of this I am working with a publisher on a "Manage Partitions with GParted" book.

Re-editing chapters of the book is taking more time than I had expected.  As such this significantly reduces the time I have to spend on development activities.  The book is slated for completion in November, so there is a point at which my life should return to normal.


Other than reviewing the code and the design approach to this enhancement, are there other questions on which you are awaiting a response?

If so, I apologize in advance.  Please feel free to ask questions and I will try to get back to you as soon as I can.
Comment 28 Curtis Gedak 2012-10-09 20:22:53 UTC
Today I got a chance to review the first 9 of 20 commits in the patch
set from comment #7.

Good things I noted about these first 9 commits are:

1)  Improves readability and maintainability of code by removing
    "global" nature of variables like lp_device, lp_disk,
    lp_partition.

2)  Comments added to existing code method set_device_partitions helps
    with maintainability of code.

3)  Large method set_device_partitions has portion extracted for new
    method parse_devices.  Smaller methods are easier to read for me.

There were only two minor format suggestions that I have.  Neither of
these two suggestions are documented anywhere yet.  I do realize that
GParted contains many different formatting styles, so I try not to be
too demanding on how the code is formatted.

A)  Please try to use tabs for the indent level, and spaces after the
    indent level to make parameters line up.  When tabs are used for
    both of these purposes the code alignment can become skewed
    depending upon whether a tab is equivalent to 2, 4, 8, or some
    other number of spaces.

    This is evident in the additional parameters in the methods:
       GParted_Core::open_device_and_disk
    and
       GParted_Core::get_filesystem
    Where it is preferred to use spaces to line up the parameters on
    the additional line because there is no indent level.

B)  If no changes to a line, then leave the black space before the
    terminating semicolon on the line.  I don't have a preference with
    regards to whether a space is used or not, but I don't think it
    makes sense to modify a line just to remove the space.

I look forward to continuing the review of your patch set during the
week of October 16th.

Would you be able to rebase the patch set to the current code in git?

If not, then no worries, I can look into doing this when I have a
chance.
Comment 29 Matthias Gehre 2012-10-10 08:56:41 UTC
Created attachment 226157 [details] [review]
LUKS patchset v8

Take your time!

Changes:
- Rebased on master
Comment 30 Curtis Gedak 2012-10-21 20:16:22 UTC
Hi Matthias,

Thank you for your patience.  Today I had some more free time and did
an initial review of the remaining patches 10 through 20 of 20.
Following are my thoughts.

In comment #24, under point 2.2 you mention that this patch set
enables nested layers of partitions under devices under partitions
under devices....  This might be a good thing, but there are some
implications to consider as well.

1)  These multiple levels might cause difficulty with other code in
    GParted, such as that for aligning partitions and ensuring that
    partitions do not overlap.

    Of course this code could probably be updated to handle the nested
    layers too.

2)  The code for the single nested layer (.logicals) was originally
    conceived for extended partitions only.  As such checks for type =
    extended partition, or .logicals() .empty() meant the same thing.

    We will need to be mindful of this as some of the existing code
    might check for the existence of a non-empty .logicals() structure
    assuming that these only exist under extended partitions.


From my preliminary review of the patch set I have the following two
suggestions:

a)  In patch 10/20, the indentation is off in the
    GParted_Core::set_device_partitions method.  See comment #28,
    point A for details on lining up method parameters.

b)  In patch 14/20, the check for FS_LUKS has been removed, but the
    corresponding comment about "Skip luks and unknown..." has not
    been similarly updated.

Overall the code is looking good, and these two suggestions are fairly
minor.  :-)


I was planning to review this more in-depth when I encountered the
following problem during testing.

There appears to be a bug that causes a crash when scanning a device
with an extended partition.

Steps to recreate:

  i)  Run GParted with this patch set and queue an operation to create
      an extended partition.
 ii)  Apply the operation.
iii)  When GParted rescans the devices it crashes.

This crash might be related to the change in use of .logicals() in point
(2) mentioned above.

Does GParted also crash on your system when extended partitions are
present?
Comment 31 Curtis Gedak 2012-10-21 20:35:42 UTC
Oops, my mistake.  An empty extended partition does not seem to cause
the crash.  At least one logical partition is needed inside the
extended partition to cause the crash.  As such insert the following
step in between (i) and (ii) so that the series of steps is as
follows:

  i)  Run GParted with this patch set and queue an operation to create
      an extended partition.
 ii)  Queue an operation to create a logical partition.
iii)  Apply the operation.
 iv)  When GParted rescans the devices it crashes.
Comment 32 Matthias Gehre 2012-10-22 11:46:04 UTC
Created attachment 226981 [details] [review]
LUKS patchset v9

This patchset:
1. is rebased on current master
2. fixes the two formatting/comment issues from comment #30 inside their respective commits
3. fixes the bug from comment #30/#31 by adding two more commits:
  Previously, in refresh_combo_devices() we would call
  liststore_devices ->clear(), which in turn would call
  the combo_devices_changed() handler with an active device index -1
  (because the list of device was just cleared).
  combo_devices_changed() would then call Fill_Label_Device_Info() which
  crashed without valid device.
  Now we deactivate the signal handler before clearing the list.

  I also modified combo_devices_changed() to set the currently selected device to   the first one in the list if none was selected.
  
  The last commit is more cosmetic.
Comment 33 Curtis Gedak 2012-10-27 20:25:24 UTC
Thank you Matthias for the updated patch.

Since your patch set is important, I have asked Mike Fleetwood to take a look at it too.

Mike has been developing patches and enhancements for GParted for over a year and is well acquainted with the code base.

I should be able to spend more time on this in early November as I am nearing completion of my book on GParted.
Comment 34 Mike Fleetwood 2012-10-29 14:36:40 UTC
Hi Matthias,

Firstly, thank you for your efforts so far.

I have been reviewing your code.  There is a lot of changes, so to make
reviewing easier I would like to break the task up into multiple parts,
reviewing just the first part now and leave the rest for afterwards.

I see that the first 6 patches were originally from bug 683149.  So I
would like to take those first 6 patches from LUKS patchset v9 in
comment 32 and review them back in there original bug.  It makes sense
to me because the changes are stand alone and when looking back at the
history the review of those 6 patches will be in that bug report.

So off to bug 683149 ...

Thanks,
Mike
Comment 35 Mike Fleetwood 2012-11-11 12:57:43 UTC
Hi Matthias,

We can resume looking at the rest of the LUKS support now.  Please
submit an updated patchset when your ready.  One extra point from my
earlier quick scanning of the previous patchset is that I would
appreciate if some of the commit messages could explain more so that I
can answer these type of questions:
* What incremental functionality is this change providing?
* How does this change help the over all goal?
* Why is this change necessary?
* How does it work differently?

This all helps me get to a better understanding of the issues you have
encountered and the decisions you taken in producing the code.

Thanks,
Mike
Comment 36 Matthias Gehre 2012-11-26 04:23:33 UTC
Created attachment 229871 [details] [review]
LUKS patchset v10

Hi,

I rebased againt current master, cleaned some commits, added more descriptions
to the log messages and change some order of commits.

Please let me know if anything else needs to be done.

Best wishes,
Matthias
Comment 37 Curtis Gedak 2013-01-03 21:17:30 UTC
Hi Matthias and Mike, I haven't forgotten about this patch set.  In
fact I have been thinking about this enhancement on an off for quite a
while now.

I keep running into difficulty with how to extend this implementation.
Also I have a nagging suspicion in the back of my mind that using
logicals to store the file system for an encrypted partition is not
the right approach.  Moreover I struggle with how to grow encrypted
partitions.

This leads me to think that perhaps we need to look at this
enhancement from a new angle.  Following are my thoughts while taking
a new perspective on how to implement LUKS support.

GPARTED BASE ELEMENTS
---------------------
If we start with the basic structure of GParted, the base elements
are:

1) DEVICES - these are block addressable chunks of space that can be
   partitioned.

2) PARTITIONS - these are defined boundaries within a device, that are
   tracked within a partition table, and the area within the boundary
   can contain some type of formatting or file system that enables an
   OS to make use of the space.

3) FILE SYSTEMS - these are a defined way to format space so that an
   OS can store and access data, more specifically files.

MAPPING LVM2 PV TO GPARTED BASE ELEMENTS
----------------------------------------
When Mike implemented LVM2 Physical Volume support, each LVM2 PV was
treated as a partition.  This mapping works well for LMV2 PV support
and enables growing, shrinking, and moving.

When thinking about how to extend GParted with LVM2 Logical Volume
support, we can foresee that this is possible with Mike's chosen
implementation.  Specifically, PVs are used to make up a Volume Group.
A VG is a block addressable chunk of space, so this is similar to a
Device.  Within each VG, several Logical Volumes can be created.  An
LV is defined boundaries within a VG, so this is similar to a
partition.  Continuing with this analogy, each LV can be formatted
with a file system.  Hence the implementation for LVM2 PVs looks like
it could be extended to fully support LVM2 LVs.

This is not to say that such an extension is simple to do.  In fact I
think it might be complicated, and might involve creating an
LVM2_VG_Core method, similar to the GParted_Core method currently used
for handling partitioning.

MAPPING LUKS TO GPARTED BASE ELEMENTS
-------------------------------------
Now let us focus on LUKS support in GParted.

Where does crypt-LUKS fit into the GParted base elements?
Is LUKS similar to a device, partition, or file system?
Or is LUKS something else?

If I understand correctly, crypt-LUKS can be used to encrypt disk
devices in addition to partitions.  The resulting encrypted area is
available as a block device via /dev/mapper/[device-label].  This
leads me to think that LUKS works more like a device than a partition.

In GParted, the closest thing to LUKS is probably DMRAID.  Both of
these present space available as a block device via
/dev/mapper/[device-label].  Both require external commands to create
and remove this device mapping.  This leads me to think that LUKS
should be treated more like a device, than like a partition.

With this in mind, the contents of a LUKS encrypted partition should
likely be treated like a device, and might be similar in nature to how
we might add LVM2 LV support.

Those are my thoughts at the current time.  I am still unsure as to
how to implement LUKS support such that we can grow encrypted
partitions.


What are your thoughts on how to implement LUKS so that we can support
growing encrypted partitions?
Comment 38 Matthias Gehre 2013-01-03 23:41:12 UTC
Hi Curtis and Mike,

You are right that the data contained in a LUKS container can be formatted like a device and thus falls in the DEVICES category.
But most users directly format their LUKS mapping with some file system.
libparted represents that as a "loop" partition table with one PARTITION,
so no problem here.
The LUKS mapping is implemented as a (hidden) DEVICE in the current patchset.
In luks::set_contained_partition, GParted_Core::parse_device is used to
fill it.


For growing:

Let a LUKS container be on /dev/sda5 and it's mapping be /dev/mapper/data
To grow an encrypted partition, one must
(1. grow the partition /dev/sda5 by means already provided by gparted)
2. User unlocks the LUKS partition /dev/sda5 (which creates the mapping /dev/mapper/data) (already in the patchset)
3. User clicks "grow" on /dev/mapper/data. Implementation would just call "cryptsetup resize data".
(4. grow whatever is inside /dev/mapper/data)

For visual representation:

I would not separate the LUKS partition (/dev/sda5) from its mapping /dev/mapper/data too much. The system represents it as two block devices,
but from a users perspective they really behave like one.

I think of it that way: By unlocking a LUKS partition, the user actually "fills" /dev/sda5 by /dev/mapper/data. In a perfect world, we would not have
/dev/mapper/data at all: Unlocking a LUKS partition would just change what we see through /dev/sda5. And this is the way I propose we present it to the user.

Strictly following the current gparted implementation,
the mapping /dev/mapper/data would only appear in the devices dropdown.
I find this cumbersome, because then I can only view the DEVICE /dev/sda or
/dev/mapper/data, making the relation between both not very obvious.
This becomes a bigger problem with more LUKS containers/LVM2.

My approach in the patchset is to put multiple devices (/dev/sda and /dev/mapper/data) into one treeview. Because the treeview is built from PARTITIONS and their logicals, I used that for LUKS.
In the GUI, LUKS then looks like an extended partition (/dev/sda5) with various other partitions/file systems inside (those on /dev/mapper/data).

So to do the GUI thing, I use logicals. More specifically:
I copy the partitions from the DEVICE /dev/mapper/data to logicals
of the extended partition /dev/sda5.
This also feels like a hack to me.

A cleaner solution would be a fourth type of partition after "primary/extended/logical" of type "container". It would not have a file system like primary/logical partition or a list of logicals like extended partition, but carry a reference to a DEVICE.
Then /dev/sda5 would be of type "container partition", and reference the DEVICE of /dev/mapper/data.
This could also be used in LVM2 for pv, which would reference their vg DEVICE.

I'm sorry for the long text, I didn't have enough time to make it shorter.
Comment 39 Curtis Gedak 2013-01-07 20:52:49 UTC
Created attachment 232940 [details]
GParted encrypted partitions screen shot

Hi Matthias,

Thank you for your detailed response.  Following are some thoughts on
the high-level approach to supporting LUKS, a constraint we face with
resizing active partitions, and some tests using the
current patches from comment #36.


HIGH LEVEL APPROACH TO SUPPORTING LUKS
--------------------------------------
I think we agree that the data contained in a LUKS container falls
into the DEVICES category.

With this realization, there are many different configurations that
could occur on a physical device.  In fact I might say there are an
infinite number of possibilities due to the recursive nature of using
device mapping.  Some practical ones that that I can think of are:

  - entire device encrypted containing a single file system
  - entire device encrypted containing a partition table
  - entire device encrypted containing an LVM2 PV
  - device contains partition table with encrypted partitions
  - device contains LVM2 PV used in a Volume Group with encrypted LVs
  - device used in RAID containing partition table with encrypted partitions.
  ... and so on.

Ideally GParted should be able to handle all situations, but I would
be happy if it handled the most commonly used ones.  For instance I
don't think it makes much sense to encrypt a partition that is
contained on a device which is entirely encrypted.


ACTIVE PARTITION RESIZING CONSTRAINT
------------------------------------
One constraint is that partition boundaries cannot be changed if the
partition is in use.  This constraint might be lifted with Linux kernel
3.6 on-line partition resizing with the addition of
BLKPG_RESIZE_PARTITION; however, we do need to work with commonly
supported kernels.

As such we will need a way to activate and deactivate LUKS within
GParted in order to grow an encrypted partition.

Do you have some ideas on how to accomplish activating and
deactivating LUKS in GParted?


TESTING USING CURRENT PATCHES
-----------------------------
I wanted to test some of the possibilities using the patches in
comment #36.  Specifically I wanted to test:

  A) Encrypted primary partition containing formatted partitions
  B) Encrypted logical partition containing a file system

I performed these tests using Ubuntu 12.10 virtual machine on a 2
GiB virtual disk device.  Following are the steps I used to create
these two scenarios:

1)  Using GParted:
      Create primary sdb1 512 MiB unformatted partition

2)  Setup LUKS encryption on sdb1

  $ sudo cryptsetup luksFormat /dev/sdb1 -c aes -s 256 -h sha256
  $ sudo cryptsetup luksOpen /dev/sdb1 vault

3)  Using GParted:
      Create MSDOS partition table inside encrypted sdb1 partition
      Create primary vault1 256 Mib ext2 partition "crypt-ext2"
      Create primary vault2 253 MiB ext3 partition "crypt-ext3"

4)  Using GParted:
      Create extended partition sdb2 using all unallocated space
      Create logical sdb5 256 MiB unformatted partition

5)  Setup LUKS encryption on sdb5

  $ sudo cryptsetup luksFormat /dev/sdb5 -c aes -s 256 -h sha256
  $ sudo cryptsetup luksOpen /dev/sdb5 vault-two

6)  Format encrypted sdb5 partition

  $ sudo mke2fs -j /dev/mapper/vault-two -L myLabel

7)  Show /dev/mapper directory

  $ ls /dev/mapper
  control  vault  vault1  vault2  vault-two

8)  Show blkid output

  $ sudo blkid
  /dev/sda1: UUID="09acce22-9595-43b5-81ce-48d38d054bef" TYPE="ext4"
  /dev/sda5: UUID="62ce5fba-9597-4829-81b1-df0f200aca92" TYPE="swap"
  /dev/sdb1: UUID="ffdb45d0-ba9e-4bac-b1c6-e99921741816" TYPE="crypto_LUKS"
  /dev/mapper/vault1: LABEL="crypt-ext2"
    UUID="4e5715d9-9871-4733-993e-e94f32b1df2f" TYPE="ext2"
  /dev/mapper/vault2: LABEL="crypt-ext3"
    UUID="fb0923ae-1df1-4374-88f5-328c204bb937" SEC_TYPE="ext2" TYPE="ext3"
  /dev/sdb5: UUID="6aa5c5b0-a182-4502-9920-5744c0772c11" TYPE="crypto_LUKS"
  /dev/mapper/vault-two: LABEL="myLabel"
    UUID="61404c82-15af-4459-be7e-8b337dbfc790" SEC_TYPE="ext2" TYPE="ext3"

9)  Display how these tests are shown in GParted.
    (see attachment)


As can be seen in the attached screen shot, there are some issues
outstanding with the patch set for read-only support of LUKS.

i1)  There should not be an exclamation point beside sdb1.
     The warning text in partition information dialog states:

        Unable to read the contents of this file system!
        Because of this, some operating may be unavailable

     I suspect the reason is that it is assumed that LUKS encrypted
     partitions only contain file systems, and not partition tables.

i2)  There should not be a vertical white line separating vault1 and
     vault2.

     I suspect the reason is that space must be allocated for the
     Extended Boot Record in front of the logical partition.

i3)  The file system in logical partition sdb5 is not recognized

     I suspect the reason is that it is assumed that only primary
     partitions will be encrypted.

I think that some of these issues arise due to the approach to
handling LUKS.


MY THOUGHTS
-----------
I still suspect that storing the file system in logicals for
encrypted partitions does not map well to the practical encryption
scenarios we will need to support.

One problem that arises is that a user can shrink a file system within
an encrypted partition, however the encrypted partition will not be
similarly shrunk.

Perhaps your suggestion of adding a type "container" might alleviate
some of these problems.  Having said that, I still don't have a good
alternative to suggest for how LUKS should be handled.
Comment 40 Matthias Gehre 2013-02-09 04:31:07 UTC
I'm sorry for the huge delay. I'm _not_ abandoning this. I'll have more time during the next days/weeks and will come back to this issue then.
Comment 41 Matthias Gehre 2013-02-11 00:11:30 UTC
Created attachment 235654 [details]
Screenshot showing same setting

i1)
The problem you encountered was due to not setting fs.onlineread, which was introduced some commits ago.
I updated commit "Add initial LUKS support (#627701)" in the patchset to do that. Now the exclamation points disappear.

i2) I see the yellow line separating vault1 and vault2, but I don't understand how that can relate to the extended boot record. The Right-Click->Information Menu shows me, that vault1 and vault2 are adjacent (last sector of vault1: 1026047, first sector of vault2: 1026048)

i3)
I cannot reproduce that, see my screenshot.
Comment 42 Matthias Gehre 2013-02-11 00:12:56 UTC
Created attachment 235655 [details] [review]
LUKS patchset v11
Comment 43 Matthias Gehre 2013-02-11 00:30:43 UTC
> One problem that arises is that a user can shrink a file system within
> an encrypted partition, however the encrypted partition will not be
> similarly shrunk.
- This will be done separately. First you can shrink the file system (already works)
by clicking on one of the lines "/dev/mapper/vault1", "/dev/mapper/vault2" or "/dev/mapper/vault-two" in the screenshot and then Resize in the Menu.

- Then (not yet implemented, will come in followup patchset after this one got merged), one will click on one of the lines "/dev/sdb1" or "/dev/sdb5" and choose Resize from the Menu to resize the Luks container together with the partition it resides on.
So /dev/sdb5 and its LUKS container are resized at the same time. This is sane, because the LUKS container has no "size" in it's header. It automatically takes all of the underlaying partition. Thus one must resize the partition to "resize" the LUKS container.
If the LUKS container is already open, it has a temporary "size" parameter, which allows us to resize the open container without closing it first, then resizing the underlaying partition. GParted will have to make sure that we do not shrink the LUKS container and our partition smaller then the contained data.
Note that online-resizing of an open LUKS container is not persistent: Once closed, it will take up the whole underlaying partition upon reopen.
Comment 44 Ely Lizaire 2014-05-04 04:20:06 UTC
It's been a year and not sure how things are progression nor what is needed. The pages https://help.ubuntu.com/community/ResizeEncryptedPartitions and http://ubuntuforums.org/showthread.php?p=4530641 may be of use for those working on this I hope.
Comment 45 Roc Vallès 2014-05-08 20:01:42 UTC
This functionality is indeed very desirable.

Right now, resizing encrypted partitions is a nightmare.
Comment 46 Liv 2015-01-23 21:32:24 UTC
In the post-Snowden world, it would be awesome if a popular solution like GParted could handle creating and modifying encrypted partitions. Is there any chance that the existing patch could be polished and finally included in a release?
Comment 47 Matthias Gehre 2015-01-25 19:28:19 UTC
I'm sorry, but I'm no longer interested in porting the patches to the newest gparted version. Someone else may take over.
Comment 48 Mike Fleetwood 2015-03-06 08:21:38 UTC
Hi All,

I am evaluating what needs to be done to add LUKS support to GParted.  I
would appreciate if you could reply to the following GParted forum post
with the requested details:

Feature Requests --> What storage layout do LUKS users have?
http://gparted-forum.surf4.info/viewtopic.php?id=17329

Thanks,
Mike
Comment 49 Liv 2015-03-06 10:14:43 UTC
Hi Mike, 
Thanks a lot for looking into this. I'll answer your question, if you don't mind. 

My personal set-ups are quite very simple, generally. It's either having a dedicated encrypted partition with ext2/ext3 FS on the computer hard-drive, or a USB Key with one FAT32 partition for maximum compatibility, and a LUKS encrypted partition. 

geek@liv-inspiron:~$ lsblk 
NAME                                                 MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINT
[..]
sdc                                                    8:32   1   7.5G  0 disk  
├─sdc1                                                 8:33   1   3.6G  0 part  /media/geek/Transcend
└─sdc2                                                 8:34   1   3.9G  0 part  
  └─luks-39b9ce12-f641-4fb6-b375-d07d8b0f2f14 (dm-0) 252:0    0   3.9G  0 crypt /media/geek/USB Device

I may consider more exotic setups in the future, but for now I'm simply hoping to have an easy way to setup a dedicated LUKS-encrypted partition to store sensitive data, backups, etc.
Comment 50 Liv 2015-03-06 10:17:04 UTC
A huge bonus would be to have a means to setup encrypted partitions that are compatible cross-platform, i.e. can be in default setups accessed on Windows/Mac OS X, and not only on Linux...
Comment 51 Mike Fleetwood 2015-03-12 21:40:53 UTC
Hi Liviu,

The scope of what I am investigating is only supporting LUKS encryption.
That is the command 'cryptsetup isLuks /dev/MY_ENCRYPTED_VOL' reports
true.

From a quick search DoxBox will allow access to LUKS encrypted volumes
on Windows.  Didn't see any direct way to access such a volume on
Mac OS X though.

Mike
Comment 52 Mike Fleetwood 2015-03-27 20:54:51 UTC
Proposed scope of LUKS support
------------------------------

From the responses I have received [1] and the research I have done [2],
I am proposing that GParted should support LUKS encryption with the
following constraints:

1) Only supported encryption using a LUKS header.
   (Required for automatic detection)

   It is possible to use cryptsetup to create dm-crypt mappings without
   using a LUKS header or even multiple mappings in one partition, but
   this is not done and not supported by the boot process used by
   distributions.  It is not possible to autodetect this so therefore
   is not considered supportable.

2) Only supports one-to-one-to-one mapping of partition to encrypted
   volume to file system.

   It is possible to create partition tables such as MSDOS or GPT within
   encrypted volumes.  This has not been observed, and no documentation
   has been seen describing this.  It is doubtful that the boot process
   supports this.  Therefore it will not be supported.

   LVM2 Volume Manager will only be supported to the extent that GParted
   currently supports it.  That is only LVM2 Physical Volumes will be
   supported as file systems.  Supporting for Volume Groups and Logical
   Volumes currently doesn't exist in GParted and it is outside the
   scope of this change to add it.  Therefore encrypted partitions
   inside Logical Volumes will not be possible to support.

[1] Feature Requests --> What storage layout do LUKS users have?
http://gparted-forum.surf4.info/viewtopic.php?id=17329

[2] Performed default encrypted installation of Fedora 21, Debian 7.6,
openSUSE 13.2 and Ubuntu 14.10 LTS.  They all produce setups like this
(modulo minor partition numbering, object naming and file system choice
differences):

device          content     mount point
+ ptn1          ext4        /boot
+ ptn2          LUKS
  + cryptname   LVM2 PV
    + rootlv    ext/btrfs   /
    + swaplv    swap

Thoughts welcome,
Mike
Comment 53 Curtis Gedak 2015-03-28 16:19:22 UTC
Hi Mike,

Your proposal sounds good.  Support for the "simple" encrypted LVM setup you described would be a step forward, and appreciated by users.

One caution is that there are a some technical and design hurdles to overcome in order to support resizing encrypted partitions.  At the time Matthias was developing a patch set, I was unable to come up with a solution to a problem with resizing.

If I recall correctly the encrypted file system needs to be resized while online, but the partition had to be resized offline.  The challenge here was that if the encrypted file system was brought online in a larger than needed partition, the encrypted file system assumed it used the whole partition.  So in my mind the two resize actions needed to be performed at the same time (for example shrink encrypted file system and then immediately shrink partition).  I think that this might now be possible with the online partition resize capability available with libparted-3.2.

Curtis
Comment 54 Mike Fleetwood 2015-03-29 10:15:47 UTC
Hi Curtis,

(In reply to Curtis Gedak from comment #53)
> ...  Support for the "simple" encrypted LVM setup you
> described would be a step forward, and appreciated by users.

So far I have only explained the constraints of my proposal and not
what I am actually proposing needs to be implemented.  So ...


Proposed details of LUKS support
--------------------------------

LUKS support needs to be like an optional encryption capability of the
partition.  (This is another reason for the one-to-one-to-one mapping
constraint).  It will be mostly automatically managed with the
partition.

Details of the operation behaviours:

1) Refresh Devices -
   LUKS volumes will be left in the current open or closed state.  Open
   LUKS volumes will have their content detected and queried just as
   non-encrptyed partitions do.  Closed volumes can only be detected as
   containing LUKS encrypted data.

2) New -
   Create new partition dialog will need these widgets adding:
   * Encryption tick box
   * LUKS passphrase entry box (only active when above ticked)
   LUKS volume will be created when requested and left open.

2) Resize -
   Will automatically resize the LUKS volume with the partition.  The
   LUKS volume must be open to be resized.  Either it must already be
   open, or GParted must know the passphrase to be able to open it.

3) Move -
   Will move the LUKS volume just as partition contents.  This will
   require the LUKS volume being closed.  Should re-open the LUKS
   volume when finished if it was open before performing the move.
   This will require knowing the passphrase.

4) Copy -
   Copying into unallocated space and creating a new partition will
   create a LUKS volume if and only if the source is in a LUKS volume.
   Copying into an existing partition will be performed at the file
   system content level allowing the data to be moved into, out of or
   maintaining encryption depending on whether the destination, source
   or both partitions are encrypted.  When copying into an encrypted
   partition the LUKS volume must be open first.

5) Format -
   To format an encrypted partition the LUKS volume must be open first.
   Question:
   Q1) How to add or remove encryption to partition?

5) Open / Close encrypted volume                         -
   Mount/Unmount (swapon/swapoff or activate/deactivate) -
   This is now a two stage process for encrypted partitions:
    1) | open LUKS              ^ close LUKS
    2) v mount file system      | unmount file system
   Need an additional item on the partition menu, named Open encrypted
   volume / Close encrypted volume.  Must be an additional menu item
   so that in the open encrypted volume and file system not mounted
   state both Close encrypted volume and Mount on can be shown.

6) Check -
   The LUKS volume must be opern before the operation can be performed.
   It will ensure the LUKS volume is maximized just before the file
   system is maximized.

7) Label file system -
   New UUID          -
   The LUKS volume must be open before these operations can be
   performed.

8) Information -
   Add following fields to the information dialog:
      Encryption
        UUID:   12345678-9abc-def0-1234-56789abcdef0
        Status: Closed | Open
        Path:   /dev/mapper/sdb5_crypt
   Optionally add Used, Unused, Unallocated and Size fields on the
   right hand side too.
   Question:
   Q2) How to graphically represent open LUKS volume with recognised
       content?


Q1) How to add or remove encryption to a partition?
Options:
* Require the partition to be deleted and re-created with or without
  encryption as required.
* Have format remove it when the encrypted volume is not open.
  (Not this.  The UI presents no clues that formatting a partition when
  the volume is closed will remove the encryption layer).
* Have new partition menu item to Add encryption or Remove encryption
  depending.


Q2) How to graphically represent open LUKS volume with recognised
    content?
    In the main window, Information dialog and all the manipulation
    dialogs.
Options:
* Double border: LUKS purple outer, file system inner.
* Lock symbol over one of the corners of the graphic.
* Something else.


This feels like a lot to do.  Larger than anything I have tackled so
far with GParted.  Larger than LVM2 PVs, larger than whole disk
devices.  It will take a while.  Will have to split it into chunks.


Thanks,
Mike
Comment 55 Curtis Gedak 2015-03-29 15:58:28 UTC
Hi Mike,

It looks like you've done quite a bit of investigation.  I agree that this is a large undertaking.

Another issue that might arise pertains to the need to acquire and likely store the encryption password for GParted to encrypt/activate LUKS.  To address any security concerns might require investigation into how other security applications deal with this requirement.

At the moment I don't have good answers to the questions raised in comment 54.  I do think that it is important to have a good understanding of how to handle write/encrypt functionality before implementing read/decrypt.  Otherwise the read/decrypt stuff might not support write/encrypt features.  That probably sounds a bit confusing, but I'm basically trying to say that implementing read-only on LUKS does not provide users with much beyond simple LUKS detection.

Curtis
Comment 56 Mike Fleetwood 2015-03-31 10:01:11 UTC
(In reply to Curtis Gedak from comment #55)
> Another issue that might arise pertains to the need to acquire and likely
> store the encryption password for GParted to encrypt/activate LUKS.  To
> address any security concerns might require investigation into how other
> security applications deal with this requirement.


LUKS password handling, threats and preventative measures
---------------------------------------------------------


1) Password handling

GParted will ask the user for the LUKS password so that it can perform
various operations including create new LUKS volume, open LUKS volume,
etc.  Therefore GParted will at least need to save the LUKS password in
memory between operation submission and operation apply.  GParted should
probably save the LUKS password in memory for as long as GParted is
running to avoid having to prompt the user for the password for every
set of operations submitted.  GParted must clear the passwords from
memory when it is shutdown.  No other method of clearing the passwords
will be provided as GParted is a task specific tool run for a short
amount of time.

GParted must not become a password manage so it must never save LUKS
passwords to disk across separate invocations of GParted.

cryptsetup can accept a LUKS password in a number of ways:
 1) prompted on standard input:
        # cryptsetup luksOpen /dev/sdb1 sdb1_crypt
        Enter passphrase for /dev/sdb3:
 2) batch read from standard input:
        # echo -n password | cryptsetup luksOpen --key-file - /dev/sdb1 sdb1_crypt
 3) batch read from  named file:
        # echo -n password > password.bin
        # cryptsetup luksOpen --key-file password.bin /dev/sdb1 sdb1_crypt
(Echoing the password is only to illustrate how the cryptsetup command
operates and will never actually be performed as it would put the
password into the shell history and appear in the process list for a
short amount of time).

GParted should avoid writting a temporary file containing the LUKS
password as it intorduces extra complexity with trying to safely handle
and erase file content.  Instead GParted must programatically pass the
LUKS password via standard input to the cryptsetup command.


2) Threats and system preventative measures

Threats to GParted storing LUKS password in memory for the life of
GParted running are:

 1) Root privileged process reading GParted's virtual memory, optionally
    using a debugger.
 2) Cold boot attack [1] reading RAM including GParted's memory.
 3) Physical access to the hard drive to read swap after GParted's
    memory is paged out.
 4) Physical access to the hard drive to read swap after hibernating the
    machine.
 
Assuming that the LUKS volume is open a root process can simply read all
the files in the encrypted volume and can query the kernel for the
master key.
    # dmsetup table --target crypt --showkeys
These are trivial compared to trying to find the LUKS password in the
memory of the GParted process.  Also see memory encryption below as a
method of making it harder to retrieve LUKS password from memory.

Cold boot attack appears to be limited to machines with DDR2 and older
memory [2] and there are other threats to be protected against first
[3] such as preventing malware running with root privileges.  Again
memory encryption would make it harder to retrieve LUKS passwords from
memory.

Having process memory being paged out can be prevented by locking it
into RAM, but hibernate will write all memory to the swap partition
regardless.  Distribution installers when setting up encryption, encrypt
all file systems, except /boot, and the swap space.

[1] Cold boot attack
http://en.wikipedia.org/wiki/Cold_boot_attack

[2] Wipe RAM on shutdown to prevent Cold Boot Attack
http://superuser.com/questions/464297/wipe-ram-on-shut-down-to-prevent-cold-boot-attack

[3] Mitigating the LUKS decryption key residing in RAM for FDE (Full
    Disk Encryption)
http://security.stackexchange.com/questions/81525/mitigating-the-luks-decryption-key-residing-in-ram-for-fde


3) GParted coded preventative measures

GParted must only store LUKS passwords in memory and only pass via stdin
stream to the cryptsetup command.  [Hard requirement]

Memory storing passwords should be overwritten with zeros before being
freed.  [Recommended]

GParted may lock passwords into RAM using mlock() to prevent it being
paged out to swap.  To this point is everything that zuluCrypt [4] GUI
disk encryption management tool does.  [Nice to have]

GParted could encrypt the memory use to store the LUKS passwords using
symmetric encryption and only temporarily creating plain text copies
when needed.  This would make it harder, but not impossible to retrieve
LUKS password from the memory of the GParted process.
[Considered unnecessary]

[4] zuluCrypt GUI disk encryption management tool
http://mhogomchungu.github.io/zuluCrypt/


Mike
Comment 57 Mike Fleetwood 2015-05-28 20:57:16 UTC
(In reply to Mike Fleetwood from comment #56)
> Having process memory being paged out can be prevented by locking it
> into RAM, but hibernate will write all memory to the swap partition
> regardless.  Distribution installers when setting up encryption, encrypt
> all file systems, except /boot, and the swap space.

This last sentence is ambiguous.  Updated paragraph with replaced sentence
and an additional observation.

Having process memory being paged out can be prevented by locking it
into RAM, but hibernate will write all memory to the swap partition
regardless.  Distribution installers when setting up encryption, encrypt
the swap space and all file systems, except /boot.  Users who setup
encryption themselves should encrypt swap space to, to avoid this
vulnerability.

Mike
Comment 58 Curtis Gedak 2015-05-29 16:05:14 UTC
Hi Mike,

From comments #56 and #57 I believe you have done due diligence in trying to ensure the security of future encrypted volume handling in GParted.

If you need assistance with any specific features of this report request then please feel free to ask.  I might not have the answers, but I can try to help.  :-)

Curtis
Comment 59 Mike Fleetwood 2015-05-29 19:39:33 UTC
Hi Curtis,

As you know I have been following the idea of making a PartitionLUKS
derived class as my favoured implementation idea.  That implied
polymorphism of Partition objects which in C++ requires using pointers
and references to objects, and not using objects directly.  (See C++
object slicing).

GParted directly copies Partition objects all over the place.  So my
first step was to "stop copying partition objects so much".  I have
about a dozen patches or so towards this goal.  I would like to bundle
them up under a separate bug number and push them up stream.  I don't
want to take 6 months or more to write a 60 patchset monster and then
ask you to review it.

Does this sound OK?

(My next step will be to actually switch to using pointers to Partition
objects in the model of Device and Partition objects.  Still no actually
LUKS support yet).

Thanks,
Mike
Comment 60 Curtis Gedak 2015-05-30 15:34:58 UTC
(In reply to Mike Fleetwood from comment #59)
> Does this sound OK?

Yes, this sounds OK to me.  Reviewing smaller pieces is much easier to do.  :-)

Curtis
Comment 61 Mike Fleetwood 2015-05-31 20:40:50 UTC
Raised bug 750168 "Reduce the amount of copying of partition objects"
to capture this first preparatory patchset.
Comment 62 Mike Fleetwood 2015-09-18 11:08:10 UTC
Raised Bug 755214 "Refactor operation merging" to capture another
preparatory patchset.
Comment 63 Mike Fleetwood 2015-11-08 09:25:56 UTC
Raised Bug 757671 "Rework Dialog_Partition_New::Get_New_Partition() a
bit" to capture another preparatory patchset.
Comment 64 Mike Fleetwood 2015-12-21 10:52:05 UTC
Raised Bug 759726 "Implement Partition object polymorphism" to capture
the final preparatory patchset before LUKS read-only support can be
added.
Comment 65 Mike Fleetwood 2016-01-02 19:25:19 UTC
Raised Bug 760080 "Implement read-only LUKS support" to capture the
changes for this feature.
Comment 66 Mike Fleetwood 2016-07-15 13:01:10 UTC
Raised Bug 767842 "File system usage missing when tools report alternate
block device names" to resolve needed issue before LUKS read-write
support can be added.
Comment 67 Mike Fleetwood 2016-09-22 14:05:07 UTC
Raised these two bug fixes to correct minor issues with read-only LUKS
support:
Bug 771323 - GParted is showing duplicate mount points for unmounted
             encrypted file systems
Bug 771670 - Usage of active encrypted swap is not shown
Comment 68 Mike Fleetwood 2016-11-21 22:10:39 UTC
Raised Bug 774818 "Implement LUKS read-write actions NOT requiring a
passphrase" to track adding the first phase of complete read-write
support.
Comment 69 Mike Fleetwood 2016-11-22 08:28:46 UTC
In comment #54 "Proposed details of LUKS support" a couple of questions
were asked.  Here are the answers.


Q1) How to add or remove encryption to a partition?

Writing or clearing LUKS format is separate to writing or clearing the
file system within, in the same way that activating each is separate.
(Opening/Closing an encryption mapping is separate from Mounting/
Unmounting a file system).  To keep these concepts separate for the user
and because writing a LUKS format will require prompting for a
passphrase make them a separate menu item.

Fragment of the partition menu will be:

    ...
    Paste
    -----------------------
    Initialize Encryption / Clear Encryption              [New item]
    Format to             >
    -----------------------
    Open Encryption Volume / Close Encryption Volume      [New item]
    Mount / Unmount
    -----------------------
    Name Partition
    ...

Initialize Encryption item will open a dialog which will ask for the
LUKS passphrase entry.  (Both New and Initialize Encryption will have
two passphrase entry boxes as is standard practice).

Now encrypting and writing to an existing partition is a two stage
process.
 1) Initialize Encryption
 2) Format to >


Q2) How to graphically represent open LUKS volume with recognised
    content?

This has been answered and implemented in Bug 760080 "Implement
read-only LUKS support".  Add "[Encrypted] to the file system type
column.  E.g.  "[Encrypted] ext4".

Mike
Comment 70 Mike Fleetwood 2016-12-10 16:11:26 UTC
Raised Bug 775932 "Refactor mostly applying of operations" to capture
code refactoring needed in preparation for Bug 774818.
Comment 71 Mike Fleetwood 2018-04-27 20:16:14 UTC
Raised Bug 795617 "Implement opening and closing of LUKS mappings" to
capture code changes for the next step in adding LUKS support to
GParted.
Comment 72 André Klapper 2020-11-13 10:40:54 UTC
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports and feature requests in GNOME Bugzilla which have not seen updates for a long time.

If you still use gparted and if you still see this bug / want this feature in a recent and currently supported version, then please feel free to report it at
https://gitlab.gnome.org/GNOME/gparted/-/issues/
by following the guidelines at
https://wiki.gnome.org/Community/GettingInTouch/BugReportingGuidelines

Thank you for creating this report and we are sorry it could not be implemented so far (volunteer workforce and time is limited).