Reduce the time taken by the GitLab Continuous Integration jobs by
parallelising make to use all available CPUs in the Docker CI image
when it is building GParted code. This includes 'make diskcheck'
because that also does a second build of the GParted code in a separate
subdirectory.
Closes!6 - Reduce the time taken by the GitLab CI jobs
The CUSTOM_TEXT enumeration is exclusively used as the type of one of
the parameters to the functions get_generic_text() and get_custom_text()
in the FileSystem class and derived classes. The definition of the
enumeration therefore belongs in FileSystem.h. Move it.
This is functionally identical, but is just to follow established coding
pattern [1] of specifying the FSType when constructing struct FS, rather
and setting it afterwards. luks.cc was added after the aforementioned
commit, but was being developed in parallel so was created [2] following
the old coding pattern.
[1] 1a4cefb960
Initialise all struct FS members
[2] 070d734e57
Add busy detection of LUKS mapping (#760080)
Prepare the GitLab Continuous Integration configuration for also
building and testing GParted on a Ubuntu image. The definition of the
image and before_script, which so far specify the CentOS Docker image
and how to install the required RPM packages, need to move from being
top level nodes to being defined per job. Namely within jobs
'centos_build' and 'centos_test'.
To avoid duplicating various nodes within multiple jobs, YAML anchors
(&LABEL) and references (*LABEL) are used. They are defined in ignored
jobs, job names starting with a dot (.).
Closes!4 - Add GitLab CI jobs to build and test GParted
Ready for adding additional Continuous Integration jobs using different
distribution Docker images. Rename thus:
build -> centos_build
test -> centos_test
Closes!4 - Add GitLab CI jobs to build and test GParted
Fragment of the tests/test-suite.log from the Docker CI image showing
details of the unit test failure:
Running main() from gtest_main.cc
[==========] Running 26 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 26 tests from BlockSpecialTest
...
[ RUN ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
test_BlockSpecial.cc:137: Failure
Failed
get_link_name(): Failed to open directory '/dev/disk/by-id'
test_BlockSpecial.cc:168: Failure
Failed
follow_link_name(): Failed to resolve symbolic link ''
test_BlockSpecial.cc:255: Failure
Expected: (lnk.m_name.c_str()) != (bs.m_name.c_str()), actual: "" vs ""
[ FAILED ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (0 ms)
...
[==========] 26 tests from 1 test case ran. (1 ms total)
[ PASSED ] 25 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
1 FAILED TEST
So the code is trying to find a symbolic link to a block device to use
in the test. It is trying to read the directory /dev/disk/by-id to find
a symbolic link, but the directory doesn't exist in the Docker CI image.
The used directory was recently changed [1] to use one which existed on
all distributions. Docker images don't even have the /dev/disk
directory. Exclude just this specific test.
[1] 7fe4148074
Use /dev/disk/by-id/ to get device symlink in test_BlockSpecial
Closes!4 - Add GitLab CI jobs to build and test GParted
Recursively list all the files below /dev as part of the 'test' job as
certain block device names are needed by the failing test_BlockSpecial
unit test.
The artifact captures all the files from the directory in which the CI
script runs to build and test GParted. It creates a ZIP file which can
be downloaded after the job finishes, whether the job succeeds of fails.
This is to capture logs from the failure of the test_BlockSpecial unit
test.
Closes!4 - Add GitLab CI jobs to build and test GParted
Add GitLab Continuous Integration job named 'test' which runs the
GParted unit tests and distcheck. Note that the job starts from a fresh
official CentOS Docker image so also has to rebuild GParted too.
So far this job fails on unit test test_BlockSpecial. Fragment of the
CI job log:
make check-TESTS
make[2]: Entering directory `/builds/mfleetwo/gparted/tests'
make[3]: Entering directory `/builds/mfleetwo/gparted/tests'
PASS: test_dummy
FAIL: test_BlockSpecial
PASS: test_PasswordRAMStore
PASS: test_PipeCapture
make[4]: Entering directory `/builds/mfleetwo/gparted/tests'
make[4]: Nothing to be done for `all'.
make[4]: Leaving directory `/builds/mfleetwo/gparted/tests'
============================================================================
Testsuite summary for gparted 0.31.0-git
============================================================================
# TOTAL: 4
# PASS: 3
# SKIP: 0
# XFAIL: 0
# FAIL: 1
# XPASS: 0
# ERROR: 0
============================================================================
See tests/test-suite.log
Please report to https://bugzilla.gnome.org/enter_bug.cgi?product=gparted
============================================================================
Closes!4 - Add GitLab CI jobs to build and test GParted
Initial GitLab Continuous Integration configuration with a single job
named 'build' which just confirms GParted can be built and installed on
the latest official CentOS Docker image.
Closes!4 - Add GitLab CI jobs to build and test GParted
Back in 2009 devicekit-disks package was renamed to udisks [1]. All
supported distributions use udisks (or more recently udisks2). None
have the old devkit-disks command. Therefore remove it from the GParted
shell wrapper.
[1] https://www.freedesktop.org/wiki/Software/DeviceKit-disks/
"Note
On December 1st 2009, DeviceKit-disks was renamed to udisks. This
release is expected to appear in distributions released in the first
half of 2010."
Shrinking an LVM2 Physical Volume on CentOS 7 with the latest
lvm2 2.02.177 fails like this:
Shrink /dev/sda9 from 1.00 GiB to 768.00 MiB
* calibrate /dev/sda9
* check file system on /dev/sda9 for errors and (if possib...(SUCCESS)
* shrink file system (ERROR)
* lvm pvresize -v --setphysicalvolumesize 786432K '/dev/...(ERROR)
0 physical volume(s) resized / 1 physical volume(s) not resized
Wiping internal VG cache
Wiping cache of LVM-capable devices
/dev/sda9: Requested size 712.00 MiB is less than real size 1.00 GiB. Proceed? [y/n]:[n]
Physical Volume /dev/sda9 not resized.
This upstream change to lvm2 [1] makes pvresize prompt for confirmation
whenever the --setphysicalvolumesize option is used. (The change was
included in lvm2 2.02.171 and later, which is used in recent
distributions. The reporter found the issue on Ubuntu 18.04 LTS and I
reproduced the issue on RHEL/CentOS 7.5). The set size option has to be
used when shrinking the PV before shrinking the partition therefore fix
this issue by adding lvm common option --yes when using the set size
option.
[1] https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=cbc69f8c693edf0d1307c9447e2e66d07a04bfe9
pvresize: Prompt when non-default size supplied.
Closes#1 - Can't shrink LVM partition due to pvresize prompt
After a failed LUKS unlock attempt the password entry dialog shows the
error "Failed to open LUKS encryption". Improve the user experience by
clearing that error message at the start of the next attempt to avoid
contradictory information with the main windows status of "Opening
encryption on $PARTITION" whilst performing the next unlock attempt.
Bug 795617 - Implement opening and closing of LUKS mappings
When the wrong LUKS password is entered and the [Unlock] button clicked,
the wrong password is left in the entry box and focus remains on the
[Unlocked] button. Improve the user experience by selecting
(highlighting) the whole of the wrong password ready for deletion or
retyping and ensuring that the entry box always has focus.
Just for completeness also programmatically make the password entry box
have focus when the dialog box is created and first displayed, even
though it gets this by default.
Bug 795617 - Implement opening and closing of LUKS mappings
We previously migrated our web site from http://gparted.org to
https://gparted.org under:
bug 786707 - gparted.org does not use HTTPS
and updated URLs in the GParted Manual to match in commit:
a8172ecb04
Convert Manual links to HTTPS where possible and update version
Now update the URLs displayed in the GParted application too.
Bug 796411 - Enhancements request - URL links
The Change_UUID_Warning vectors were fat16 and ntfs class member
variables, but are only ever accessed in the get_custom_text() method.
Make them local variables in get_custom_text() instead. Static so that
references to them can be returned.
I completely missed that when reading XFS file system size and usage it
was using the '-r' read-only flag to xfs_db because it was at the end of
the string on the following line of code. Move it to the start of the
xfs_db command line, like it is when reading the file system label.
Same case as for FileSystem Label dialog before; the Partition Name
dialog only has a single line of just 2 widgets. Therefore switch to a
simpler horizontal box widget to lay them out.
The FileSystem Label dialog only has a single line of just 2 widgets; a
text label and entry box widget. There is no need to use a multi-line
capable table to hold this. Switch to a simpler horizontal box widget.
Note that this change is not related to porting to Gtk 3 and stopping
using deprecated APIs because both HBox [1] and Table [2] are deprecated
in Gtk 3.2 and Gtk 3.4 and replaced by Box with horizontal orientation
and Grid respectively.
[1] NEWS file from gtkmm 3.2, actually first released in gtkmm 3.1.6
(unstable):
https://git.gnome.org/browse/gtkmm/tree/NEWS?h=3.2.0#n91
"Gtk:
* All H* or V* specialized classes have been deprecated, to
match the deprecations in the GTK+ C API. You should now
set the orientation instead.
This includes HBox, VBox, HButtonBox, VButtonBox, HPaned,
VPaned, HScale, VScale, HSeparator, VSeparator, HScrollbar
and VScrollbar."
[2] NEWS file from gtkmm 3.4, actually first released in gtkmm 3.3.2
(unstable):
https://git.gnome.org/browse/gtkmm/tree/NEWS?h=3.4.0#n162
"* Deprecate Gtk::Table in favour of Gtk::Grid."
Replace return by value of const strings from
FileSystem::get_custom_text() and get_generic_text() because that
implies duplication of those strings. Return a reference to constant
strings instead.
Compiling (with new enough g++) produces this warning:
PasswordRAMStore.cc: In member function 'void GParted::PWStore::erase_all()':
PasswordRAMStore.cc:177:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if ( protected_mem != NULL );
^~
PasswordRAMStore.cc:193:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
memset( protected_mem, '\0', ProtectedMemSize );
^~~~~~
Looks like a stray semicolon...
Bug 796293 - Fix null pointer check accidentally disabled
Replace the insert() method (which reports an error when inserting a
password with a key which already exists) with the store() method which
replaces or inserts the password depending on whether the key already
exists or not respectively. There is also an optimisation that nothing
is changed if the password to be replaced is the same as the one already
stored. The code in Win_GParted::open_encrypted_partition() is
simplified now it doesn't have to implement this pattern of behaviour
itself.
Bug 795617 - Implement opening and closing of LUKS mappings
Reports generic GParted error "Failed to open LUKS encryption" on any
failure unlocking the partition. Choosing not to display cryptsetup
reported errors because those messages and their translations are not
under GParted control.
Bug 795617 - Implement opening and closing of LUKS mappings
The underlying C coded Gtk Entry widget is careful to zero memory after
use, allowing the widget to be safely used for password entry [1].
However the C++ method Gtk::Entry::get_text() just takes the underlying
C string from the Gtk Entry widget and copies it when constructing a
Glib::ustring for the return value [2].
So directly use the Gtk/C API to get the C string instead.
[1] https://git.gnome.org/browse/gtk+/tree/gtk/gtkentrybuffer.c?h=3.22.28#n92
See function trash_area() which zeros memory and its use in
gtk_entry_buffer_normal_insert_text(),
gtk_entry_buffer_normal_delete_text() and
gtk_entry_buffer_finalize().
[2] https://git.gnome.org/browse/gtkmm/tree/gtk/src/entry.hg?h=3.22.2#n104
_WRAP_METHOD(Glib::ustring get_text() const, gtk_entry_get_text)
https://git.gnome.org/browse/glibmm/tree/docs/internal/using_gmmproc.txt?h=2.46.1#n53
_WRAP_METHOD(Glib::ustring METHOD const, FUNC) is processed to:
Glib::ustring METHOD() const
{
return Glib::convert_const_gchar_ptr_to_ustring(
FUNC(const_cast<GtkEntry*>(gobj())));
}
https://git.gnome.org/browse/glibmm/tree/glib/glibmm/utility.h?h=2.46.1#n82
Glib::ustring convert_const_gchar_ptr_to_ustring(const char* str)
{
return (str) ? Glib::ustring(str) : Glib::ustring();
}
So Gtk::Entry::get_text() calls Glib::ustring() constructor which
copies the C string to create the Glib::ustring object returned.
Bug 795617 - Implement opening and closing of LUKS mappings
To keep password dialog open, just keep running it in a loop performing
LUKS mapping unlock attempts with the entered passphrase until it
succeeds or the dialog is cancelled or closed. This is the same model
that is already used for the File Support System dialog and how the
[Rescan For Supported Actions] button is implemented.
Also any error from attempting to open the LUKS mapping is no longer
displayed in a separate error dialog or at all. Will add some sort of
error reporting into the password entry dialog in a following commit.
Creates new method Win_GParted::open_encrypted_partition() which handles
the non UI parts of attempting to open an encrypted partition. Running
"cryptsetup luksOpen" and updating the stored passphrase as needed.
Bug 795617 - Implement opening and closing of LUKS mappings
Initial addition of a password entry dialog. Looks like:
+------------------------------------------------+
| LUKS Passphrase /dev/sdb1 |
+------------------------------------------------+
| Enter LUKS passphrase to open /dev/sdb1 |
| Passphrase: [ ] |
| |
| [ Cancel ] [ Unlock ] |
+------------------------------------------------+
A standard Gtk Dialog is used to accept the password once, with any
errors displayed in a separate error dialog afterwards. This is poor UI
design. A password dialog should remain open for all authentication
attempts and only close when successful or the dialog is cancelled or
closed. This UI design issue will be improved in following commits.
Bug 795617 - Implement opening and closing of LUKS mappings
Implement Close Encryption partition menu item.
The Open Encryption action is not yet implemented and instead reports an
error detailing the open encryption command. A dialog needs to be
written to accept the password entry and pass it to the open encryption
command.
Bug 795617 - Implement opening and closing of LUKS mappings
Add new item to the partition menu to allow the user to open and close
the LUKS mapping. However for now the menu item is always disabled and
there is no implementation behind it to actually open or close the LUKS
mapping. Fragment of the partition menu is now:
...
Format to >
-----------------
Open Encryption <- New menu item
Mount
-----------------
Name Partition
...
Has to be two separate menu items to clearly represent to the user that
LUKS mappings and file system mounting are two separate busy states.
And also in the case of an open but unmounted file system to offer both
actions; close encryption and mount file system.
The text of the menu item automatically changes similarly to how it does
for the Mount/Unmount, Swapon/Swapoff, Activate/Deactivate item
depending on the state of the LUKS mapping. For open LUKS mappings it
will show "Close Encryption" and for all other cases (closed LUKS
mapping or partition is not encrypted) "Open Encryption". Again similar
to how the default of "Mount" is shown for unallocated and unknown
partitions.
Bug 795617 - Implement opening and closing of LUKS mappings
In preparation for adding the ability to toggle the encryption busy
state (open/close the encryption volume), rename existing members to
reflect that they are related to changing the file system state. (Swap
and LVM2 Physical Volumes are handled as file systems by GParted).
class Win_GParted renaming:
MENU_TOGGLE_BUSY -> MENU_TOGGLE_FS_BUSY
allow_toggle_busy_state() -> allow_toggle_fs_busy_state()
toggle_busy_state() -> toggle_fs_busy_state()
check_toggle_busy_allowed() -> check_toggle_fs_busy_allowed()
Bug 795617 - Implement opening and closing of LUKS mappings
Replace echoing "dm" into jfs_debugfs via a shell command to directly
writing "dm" to the input of the jfs_debug command. One less use of the
shell.
Bug 795617 - Implement opening and closing of LUKS mappings
As discussed in "LUKS password handling, threats and preventative
measures" [1] GParted must be able to pass LUKS passphrases to
cryptsetup via standard input to avoid having to write passwords to the
file system and deal with additional security requirements. Therefore
add a way to write input into created child processes. For small
amounts of input, writing up to the pipe buffer capacity won't block
[2]. This is 64K on versions of Linux in any currently supported
distributions.
[1] LUKS password handling, threats and preventative measures
https://bugzilla.gnome.org/show_bug.cgi?id=627701#c56
GParted must not become a password manage so it must never save
LUKS passwords to disk across separate invocations of GParted.
...
GParted should avoid writing a temporary file containing the LUKS
password as it introduces extra complexity with trying to safely
handle and erase file content. Instead GParted must
programmatically pass the LUKS password via standard input to the
cryptsetup command.
[2] pipe(7) manual page:
Pipe capacity
A pipe has a limited capacity. If the pipe is full, then a
write(2) will block or fail, depending on whether the O_NONBLOCK
flag is set (see below). ...
In Linux versions before 2.6.11, the capacity of a pipe was the
same as the system page size (e.g., 4096 bytes on i386). Since
Linux 2.6.11, the pipe capacity is 65536 bytes.
Bug 795617 - Implement opening and closing of LUKS mappings
Use private access into the PasswordRAMStore class to directly obtain
the address of the locked memory, rather than inferring it from the
address of the first stored password. This simplifies
PasswordRAMStoreTest::SetUpTestCase() and avoids encoding most of the
implementation knowledge that the first password will be stored at the
start of the protected memory.
Bug 795617 - Implement opening and closing of LUKS mappings
Test that all passwords are zeroed by PasswordRAMStore::erase_all(), the
same method as used in the PasswordRAMStore destructor.
Bug 795617 - Implement opening and closing of LUKS mappings
Move zeroing of the locked memory into separate PWStore::erase_all()
private method. Then use this in the PWStore destructor. This is so
that zeroing of all passwords can be unit tested independently of
destructing the singleton PWStore object.
Bug 795617 - Implement opening and closing of LUKS mappings
As noted in comments:
1) This is white box testing because it uses implementation knowledge
to look through the API to the internals of the password store.
2) It is not currently possible to test that the passwords are zeroed
when the store is destroyed.
However zeroing of memory is being tested when individual passwords
are erased.
Bug 795617 - Implement opening and closing of LUKS mappings
Mike Fleetwood