GNOME Bugzilla – Bug 326553
Add support for encrypted volumes to gnome-volume-manager
Last modified: 2006-02-24 19:21:02 UTC
An effort is underway to allow hal to understand volumes encrypted using dm-crypt [1]. This effort uses LUKS [2] to store encryption parameters on disk. The end state of this effort is that when a user attaches an encrypted device to the system, the user is prompted for a passphrase and that device is then mounted. This requires a patch to gnome-volume-manager so that g-v-m recognizes an encrypted volume, prompts the user for a passphrase and provides that passphrase to hald so that hald can properly set up the associated plaintext device. More information about this project, including the status of its components may be found at [3]. [1] http://lists.freedesktop.org/archives/hal/2004-December/001423.html [2] http://luks.endorphin.org/ [3] http://www.flyn.org/easycrypto/easycrypto.html
Created attachment 57139 [details] [review] Patch to add encrypted volume support to gnome-volume-manager
Ubuntu seems to use pmount in their g-v-m version which also supports crypt (and thus it already supports crypto volumes). Is there a good reason for doing so? You can find the patch they use at ftp://ftp.ubuntulinux.org/ubuntu/pool/main/g/gnome-volume-manager/gnome-volume-manager_1.5.8-0ubuntu1.diff.gz
Tim: is there a good reason for doing what? I don't understand your question. Petullo: +gvm_ask_secret (gchar *volume) +{ + gchar *secret = NULL; + gchar label_header_text[PATH_MAX + 1]; this isn't long enough if you are going to cram more than just a path in there (which is what you do). Also... ugh, I hate gchar. oh, and use g_strdup_printf instead of a static sized string. cleaner. simpler. yea. + + gboolean secret_entered = FALSE; + + GtkWidget *dialog, *dialog_vbox, *vbox, *label_header, *hbox, + *icon_lock, *label_password, *entry_passphrase, + *dialog_action_area, *button_cancel, *button_ok; I don't think we need all these variables. also, blank lines between variable declarations... ewwww. + + g_strlcpy(label_header_text, _("Secret required for "), PATH_MAX + 1); A user isn't going to know what is meant by that... better to use "Password" rather than "Secret" + g_strlcat(label_header_text, volume, PATH_MAX + 1); + + dialog = gtk_dialog_new (); + gtk_window_set_title (GTK_WINDOW (dialog), _("Secret Required")); + gtk_window_set_type_hint (GTK_WINDOW (dialog), + GDK_WINDOW_TYPE_HINT_DIALOG); no need to set this hint, gtk_dialog_new() does that already. + vbox = gtk_vbox_new (FALSE, 12); + gtk_widget_show (vbox); + gtk_box_pack_start (GTK_BOX (dialog_vbox), vbox, TRUE, TRUE, 0); + + label_header = gtk_label_new (_(label_header_text)); + gtk_widget_show (label_header); + gtk_box_pack_start (GTK_BOX (vbox), label_header, FALSE, FALSE, 0); + + hbox = gtk_hbox_new (FALSE, 12); + gtk_widget_show (hbox); + gtk_box_pack_start (GTK_BOX (vbox), hbox, TRUE, TRUE, 0); + + icon_lock = gtk_image_new_from_stock ("gtk-dialog-authentication", + GTK_ICON_SIZE_BUTTON); + gtk_widget_show (icon_lock); + gtk_box_pack_start (GTK_BOX (hbox), icon_lock, TRUE, TRUE, 0); + + label_password = gtk_label_new (_("Secret:")); + gtk_widget_show (label_password); + gtk_box_pack_start (GTK_BOX (hbox), label_password, FALSE, FALSE, 0); ... you've gotten the packing wrong for this dialog. the hbox should be the toplevel box widget inside of the dialog->vbox (ugh, get rid of the dialog_vbox variable) because the icon is supposed to be at the same height as the primary dialog text. What you really need to do is look at my existing code to generate a HIG-compliant dialog already in manager.c and re-use that. + dialog_action_area = GTK_DIALOG (dialog)->action_area; + gtk_widget_show (dialog_action_area); + gtk_button_box_set_layout (GTK_BUTTON_BOX (dialog_action_area), + GTK_BUTTONBOX_END); + + button_cancel = gtk_button_new_from_stock ("gtk-cancel"); + gtk_widget_show (button_cancel); + gtk_dialog_add_action_widget (GTK_DIALOG (dialog), button_cancel, + GTK_RESPONSE_CANCEL); + GTK_WIDGET_SET_FLAGS (button_cancel, GTK_CAN_DEFAULT); you need to look at the GtkDialog API which would simplify all of this into like 2 lines of code :) + g_signal_connect ((gpointer) button_ok, "clicked", + G_CALLBACK (gvm_on_secret_ok_clicked), + &secret_entered); + + gtk_dialog_run (GTK_DIALOG (dialog)); + + if (secret_entered) + secret = g_strdup(gtk_entry_get_text (entry_passphrase)); + + gtk_widget_destroy (dialog); + + return secret; again, read the GtkDialog API docs, they explain that gtk_dialog_run() returns which button was clicked which removes the need for your callback functions. As a note, tho, notice how my current code that pops up dialogs does not use gtk_dialog_run(). This is because while that is running, the rest of the code blocks, which means if more hardware events occur, they are postponed until the user enetrs the password for this particular filesystem. This is a Bad Thing(tm). You'll also notice that my current dialog code handles the case where the device is removed while the dialog is up waiting for user-input, this also needs to be done in the password-prompting case, because the suer may plug in their encrypted thumb drive, get the password dialog, and then unplug the drive. When that happens, the dialog needs to just disappear. All that said, I think G-V-M is the wrong place to implement this with the advent of the new gnome-mount program that DavidZ is working on. Once gnome-mount is functional, I'm going to make g-v-m call it instead of the current hal-mount DBus methods for mounting/unmounting/ejecting. The gnome-mount program will handle prompting users for passwords.
I first should clarify that I did not write the patch. I just had a look since I got that magic sentence "In Ubuntu it just works already" and so I got curious. Besides the quality of the code it seems that parts of what pmount does is now being implemented in gnome-mount, which suggests that the Ubuntu guys are basically doing the right thing that is available today. I would just like to get more comfort in working with crypto filesystems to be able to have an average user using it. The reason for the post was just to give a hint to an alternative way how others completed the task. I still think that until gnome-mount is ready Micheal's patch should go into g-v-m to get it working for now - reverting the patch later is an easy task. But the functionality should be there today if possible - and it is possible (since a solution is already available with the supplied patch).
Thanks for the tips, Jeffrey. I must admit, my GUI code was written by glade. I will implement your suggestions. I hope to find time on Friday afternoon.
Created attachment 57311 [details] [review] Patch to add encrypted volume support to gnome-volume-manager 1. label_header_text is now dynamically allocated. 2. Removed unused variable declarations. 3. Use the term "password" not "secret." 4. Remove GDK_WINDOW_TYPE_HINT_DIALOG. 5. Fix GUI code, including remove use of gtk_dialog_run(). 6. Password prompt disappears if device removed.
Jeffrey, I'm not convinced that gnome-mount is the right place for this code. My code does not mount anything. Instead, is sets up the dm-crypt device for a volume. Once HAL detects the new dm-crypt device then /that/ is mounted. I would like to hear David Zeuthen's opinion on this. I worked with David to add crypto support to hal. Originally, we thought that the password prompt code would go into gnome-volume-manager. However, this was before gnome-mount. Again, my code does not mount anything. It only set the condition that something may be mounted. After looking at the project, I don't think my code belongs in gnome-mount.
Hi, so I think this basically belongs in gnome-mount, yes: a. we need to cover the case where the crypted volume is inserted prior to login; one thing we don't want is to get g-v-m asking for the crypto password at login - that would be annoying b. users may not have automounting enabled and it should be possible to mount/unmount the cleartext volume during your session using computer:/// in Nautilus I think this would be somewhat straightforward to implement (assume that /dev/sda1 is the crypted volume) 1. teach gnome-mount about crypto partitions, e.g. if 'gnome-mount --device /dev/sda1' is invoked gnome-mount should just check whether it's a crypted volume and if it is ask for the password and tell hald to setup the volume. Then a new volume object will appear (let's call it /dev/dm-0) and we'll wait for that one in gnome-mount and mount it (we might race with g-v-m but only one wins...) 2. similar, when doing 'gnome-umount --device /dev/dm-0' ask hald to tear down the crypto device once the unmount succeeds. In g-v-m we'll simply invoke gnome-mount for crypto volumes too as we already do for filesystem volumes.... Also, this will need a few fixes in the gnome-vfs hal bits but should be entirely possible once we got 1. and 2. implemented. Specifically when the volume haven't been decrypted we want to show the volume for /dev/sda1 but we want to hide it when it's open (because then we show /dev/dm-0). Another thing is that we probably want to decorate the icons with lock emblems (locked and unlocked). Both of these things should be straightforward. Oh, btw, on the hal list we're also looking at adding Format() / PartitionDisk() methods in HAL. For the former we definitely want to easily enable formatting encrypted volumes so a future UI shell for that makes crypto easy.
Looking at the patch in comment 6 it should be relatively easy to port to gnome-mount.. Btw, I see you use the word LUKS in some places; there's really no need to do that as hald should be able to abstract away that fact. IOW, if someone were to add support for another crypto approach the interfaces we would rely on wouldn't be dependent on LUKS at all. And all these UI bits would still work. Which is nice. Another thing is that if the passphrase you looked up from gnome-keyring doesn't work you want to prompt the user for a new one... it's not clear to me that this is what's happening... (btw... that's the approach we use in the vpnc bits for NetworkManager and it appears to work fine.)
Btw, gnome-mount is kind of ready - we're shipping a pre-0.4 in Fedora Rawhide and I'll probably release 0.4 tomorrow. There's a bunch of work left in gnome-mount before it's done [1] but at this point it's at least as good/functional/whatever as fstab-sync and pmount approaches.. [1] : UI and per-user settings mainly
Hi, so I actually got inspired and added this to gnome-mount. One notable difference is that we use GnomePasswordDialog from libgnomeui. I also implemented all the various edge cases. 2006-02-13 David Zeuthen <davidz@redhat.com> * src/gnome-mount.c: Add support for passworded media. This adds three niceties 1) when mounting the crypto volume the password dialog is spawned and, on correct password, we setup the cleartext volume (using Crypto.Setup() on HAL) and then wait until the cleartext volume appears. Then we mount the cleartext volume 2) When unmounting the crypto volume, first we unmount the cleartext volume and then we teardown the cleartext volume (using Crypto.Teardown() on HAL). 3) When unmounting the cleartext volume we also tear down this volume (using Crypto.Teardown() on HAL). * configure.in: Pull in libgnomeui and gnome-keyring-1. Require hal 0.5.7 (which is hal CVS HEAD at the moment) Btw, it would be useful if someone can test this too (you need both hal CVS HEAD of gnome-mount and hal)... - it should look like this MOUNTING A CRYPTO DEVICE [davidz@daxter hal]$ gnome-mount --block --device /dev/sda1 ** Message: Crypto volume - UDI '/org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc' ** Message: setting up /org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc for crypto ** Message: Waiting for cleartext volume backed by /org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc.. ** Message: in crypto_setup_device_added for /org/freedesktop/Hal/devices/volume_uuid_43F1_517C ** Message: in crypto_setup_device_removed for /org/freedesktop/Hal/devices/volume_uuid_43F1_517C ** Message: in crypto_setup_device_added for /org/freedesktop/Hal/devices/volume_uuid_43F1_517C ** Message: /org/freedesktop/Hal/devices/volume_uuid_43F1_517C is backed by /org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc - will mount ** Message: Mounting UDI '/org/freedesktop/Hal/devices/volume_uuid_43F1_517C' ** Message: mounting /org/freedesktop/Hal/devices/volume_uuid_43F1_517C with mount_point='Secret Stuf', fstype='', num_options=2 ** Message: option='shortname=winnt' ** Message: option='uid=500' UNMOUNTING THE CLEARTEXT DEVICE [davidz@daxter hal]$ gnome-umount --block --device /dev/dm-0 ** Message: Unmounting UDI '/org/freedesktop/Hal/devices/volume_uuid_43F1_517C' ** Message: unmounting /org/freedesktop/Hal/devices/volume_uuid_43F1_517C ** Message: tearing down /org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc for crypto UNMOUNTING THE CRYPTO DEVICE [davidz@daxter hal]$ gnome-umount --block --device /dev/sda1 ** Message: Unmounting UDI '/org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc' ** Message: unmounting /org/freedesktop/Hal/devices/volume_uuid_43F1_517C ** Message: tearing down /org/freedesktop/Hal/devices/volume_uuid_f04e3732_be7c_44d0_9513_fb1a687b4edc for crypto
FYI and in relation to comment 8, I've now submitted a patch for gnome-vfs that should make this work. Now we just need a patch for g-v-m and we're all set.
this is now in CVS