GNOME Bugzilla – Bug 652460
Widget for entering password with non-pageable memory + DH
Last modified: 2012-02-29 19:34:03 UTC
In order for gnome-keyring to prompt for passwords in gnome-shell we want to be able to get the password through all the different layers of javascript, dbus, and what have you, without it getting stuck in logs, paged memory, or other leaks. In gnome-keyring we do this by using DH, and encrypting the password in transit. Need a subclass of StEntry, which holds the password in non-pageable memory. It will also handle one side of the DH stuff.
Using another approach now that we have ClutterTextBuffer. No need to subclass StEntry or any of that strangeness.
Created attachment 207274 [details] [review] Use non-pageable memory for gnome-keyring passwords * Use a ClutterTextBuffer that allocates non-pageable memory to hold passwords for gnome-keyring, ssh, gpg * Requires gcr 3.3.5 and clutter 1.9.4
Comment on attachment 207274 [details] [review] Use non-pageable memory for gnome-keyring passwords Patch needs rebasing, getting it off the review list
Created attachment 208660 [details] [review] Rebase on new keyring prompt patch
Review of attachment 208660 [details] [review]: Looks mostly good, minor comments below: ::: src/shell-keyring-prompt.c @@ +655,3 @@ { + ClutterTextBuffer *buffer; + ClutterActor *text; Unused variable ::: src/shell-secure-text-buffer.c @@ +54,3 @@ +static const gchar * +shell_secure_text_buffer_real_get_text (ClutterTextBuffer *buffer, + gsize *n_bytes) wrong indentation @@ +75,3 @@ + guint position, + const gchar *chars, + guint n_chars) Dito. @@ +132,3 @@ +shell_secure_text_buffer_real_delete_text (ClutterTextBuffer *buffer, + guint position, + guint n_chars) ... @@ +138,3 @@ + + if (position > self->text_chars) + position = self->text_chars; Any reason for not returning 0 here? @@ +163,3 @@ + self->text_chars = 0; + self->text_bytes = 0; + self->text_size = 0; Unnecessary initialization (all instances returned by g_type_create_instance are already memset(0))
Created attachment 208680 [details] [review] Updated for review. Good catch on the n_chars :)
Review of attachment 208680 [details] [review]: Looks good, though the n_chars change is not what I meant ::: src/shell-secure-text-buffer.c @@ +140,3 @@ + { + position = self->text_chars; + n_chars = 0; Mmh, that's not what I meant - n_chars would have been set to 0 with the next condition anyway (unless it already was 0 to begin with). I was just suggesting this shortcut: if (position > self->text_chars) return 0; If you don't like that, I would actually prefer the previous patch version :) (but up to you really, neither variant is wrong)
Comment on attachment 208680 [details] [review] Updated for review. Ah right. Reverted that bit to the previous code. That way it matches what I've done elsewhere in similar buffer code for Gtk+.