GNOME Bugzilla – Bug 754028
No support for ed25519 and ecdsa SSH keys
Last modified: 2018-03-08 21:34:19 UTC
Seahorse should be updated to support ed25519 and ecdsa keys to match OpenSSH support. I'm using Fedora 22 with Seahorse 3.16.0 and I can't add SSH keys that are using ed25519 algorithm. Looking at the Git logs the situation is still actual. I've even checked in the file seahorse-ss-key-data.c and .h only RSA and DSA are supported. OpenSSH 6.9 supports RSA, DSA, ED25519 and ECDSA.
Created attachment 309945 [details] [review] ssh: Support for listing ECDSA/Ed25519 keys
Created attachment 309946 [details] [review] ssh: Support for generating ECDSA/Ed25519 keys -- (Note that this could break string freeze, as it adds new translatable strings "ECDSA" and "Ed25519" to the Glade file.)
Hi Daiki, I've applied your patch to the version 3.16.0 from Fedora, but it does not seem to work. So on one side, I can't seem to view (they are not even listed) my existing ED25519 keys. However when I try to generate them, it "work". They are actually generated in my ~/.ssh folder. But they don't appear in Seahorse. It works for ECDSA key though!
Hi again Daiki, If in your first patch (listing the keys), you replace the strcmp by strstr (like see below), then this also work. I have to admit that your "magic" code for detecting the key length, I did not understand it :-) but it seems to work. Side note (but this is another bug I suppose), the length for RSA keys are always slightly wrong on 3.16.0 (I've checked the default SRPM from Fedora, they don't seem to apply any patch). A 4096 bit key is 4112, a 2048 bit key as 2064, etc. The formula to calculate the length is: ((len - 21) * 8); If you replace 21 by 23, the results are all correct. But given that these constants don't have explanation, I can't judge if this correction makes sense or not. The corrected patch: @@ -63,6 +92,10 @@ calc_bits (guint algo, gsize len) static guint parse_algo (const gchar *type) { + if (g_str_has_prefix (type, "ecdsa")) + return SSH_ALGO_ECDSA; + if (strstr (type, "ed25519")) + return SSH_ALGO_ED25519;
One more thing Now I can view and generate ED25519 keys (and it seems to work too for ECDSA keys). But when I use ssh to login to a remote host where a ED25519 has been exchanged, it does not seem to see the key in Gnome keyring and I'm prompt for the password in the terminal. If I try the same thing with a RSA key, this work as expecting. When I connect via SSH, I get prompt for the password by seahorse and I'm able to store it in my keyring. I guess there is another part in seahorse which could require some update. If you can point me out, I could try to help.
An addition to the patch? Here is a part in the code where we test for the algorithms but where it was not updated for ED25519 and ECDSA. Here is the patch (if it makes sense): diff -Naur seahorse-3.16.0-patched1/ssh/seahorse-ssh-key-data.c seahorse-3.16.0/ssh/seahorse-ssh-key-data.c --- seahorse-3.16.0-patched1/ssh/seahorse-ssh-key-data.c 2015-09-05 20:04:40.355861520 +0200 +++ seahorse-3.16.0/ssh/seahorse-ssh-key-data.c 2015-09-05 20:01:18.245653380 +0200 @@ -239,6 +239,10 @@ secdata->algo = SSH_ALGO_RSA; else if (strstr (secdata->rawdata, " DSA ")) secdata->algo = SSH_ALGO_DSA; + else if (strstr (secdata->rawdata, " EC ")) + secdata->algo = SSH_ALGO_ECDSA; + else if (strstr (secdata->rawdata, " OPENSSH ")) + secdata->algo = SSH_ALGO_ED25519; else secdata->algo = SSH_ALGO_UNK; } As for the "ssh-agent" problem, I see that this is not a Seahorse bug anymore but a gnome-keyring one: https://bugzilla.gnome.org/show_bug.cgi?id=723274 https://bugzilla.gnome.org/show_bug.cgi?id=672145 https://bugzilla.gnome.org/show_bug.cgi?id=641082
Created attachment 310717 [details] [review] Another patch complementary of the previous ones
I am not sure if I should create a new bug report or just complete that current one. But it seems that seahorse does not support (in addition) the new storage file format. Note that ED25519 uses automatically the new file format, but one can force it using the '-o' option. I recently generated an RSA key with a length of 4096 bits, but I chose the new format and to have 100 rounds. I used the following command to generate it: ssh-keygen -o -a 100 -t rsa -b 4096 I can successfully use the key using ssh-agent. But I cannot add the key to my keyring in Gnome (it is still blurry for me where is the boundary between seahorse and gnome-keyring, so sorry if my description is not more specific).
Bumping the version to 3.18.
9 months later and still no solution in sight? I just switched to ed25519.
No support of ed25519 or ecdsa in version 3.20. Only show keys of type ecdsa because seahorse search the string "DSA" and find it in "ecDSA".
Created attachment 357626 [details] [review] Management of ssh keys of type ecdsa and ed25519
Review of attachment 357626 [details] [review]: OK, I did a sanity-check and the only obvious problem I see is overriding set_property with empty functions. Other than that, I'd be willing to commit this if Stef or another interested maintainer does not respond to object. ::: pgp/seahorse-gpgme-keyring.c @@ +923,3 @@ gobject_class = G_OBJECT_CLASS (klass); gobject_class->get_property = seahorse_gpgme_keyring_get_property; + gobject_class->set_property = seahorse_gpgme_keyring_set_property; This can't be right, you shouldn't need to override set_property with an empty function. Why do you need this? I can only imagine that it is a hack to fix some other weird bug? ::: pgp/seahorse-pgp-subkey.c @@ +288,3 @@ } FlagNames; + const FlagNames flag_names[] = { I don't think this is needed for your patch? If you changed this to avoid a compiler warning, then I'd make the change in a different patch. ::: src/seahorse-sidebar.c @@ +1430,3 @@ + SeahorseSidebar *self; + + self = g_object_new (SEAHORSE_TYPE_SIDEBAR, It looks like you changed this function to avoid a compiler warning, right? I would remove this change from your patch, since it makes the patch harder to read. Such things are best left to separate patches in GNOME. Also, I'm not sure what prevailing code style is in seahorse, but in most of GNOME we would prefer an explicit cast here: self = SEAHORSE_SIDEBAR (g_object_new (... ::: ssh/seahorse-ssh-key-data.c @@ +115,2 @@ static gboolean parse_key_blob (guchar *bytes, gsize len, SeahorseSSHKeyData *data) It looks like you only fixed the indentation in this function, right? I would remove this change from your patch, since it makes the patch harder to read. ::: ssh/seahorse-ssh-source.c @@ +367,3 @@ gobject_class->finalize = seahorse_ssh_source_finalize; gobject_class->get_property = seahorse_ssh_source_get_property; + gobject_class->set_property = seahorse_ssh_source_set_property; Same question here.
For the seahorse_sidebar_new, I have made a test to initialize a boolean and I don't have totally revert to initial version. On my Fedora 26, my first compile of seahorse give me a version where GnuPG Keys and OpenSSH Keys aren't visible. Take a while to discover that I can click on the "invisible" part and see my ssh keys (only after some modification and recompilation). If set_property is null, then object aren't correctly initialized and stay invisible with a g_assert not valid -> use of return in a function witch change the behavior of this function (sorry, I don't retrieve the loop and the name of the function). I have some other enhancement and correction concerning the creation of ecdsa keys...
Created attachment 357647 [details] [review] Management of ssh keys of type ecdsa and ed25519
Review of attachment 357647 [details] [review]: ::: pgp/seahorse-gpgme-keyring.c @@ +867,3 @@ +seahorse_gpgme_keyring_set_property (GObject *object, guint prop_id, const GValue *value, + GParamSpec *pspec) +{ The empty set_property function is still my main concern. What happens if you add a call to G_OBJECT_WARN_INVALID_PROPERTY_ID() here? Something is wrong; you shouldn't need to have a set_property function if you have no properties. At any rate, I suspect this is a separate, unrelated bug, right?
With : static void seahorse_gpgme_keyring_set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); } plus make, and attempt to create a pgp key (never expires + with expire date) ??????:~$LANG=C ./seahorse (seahorse:20931): GLib-GObject-CRITICAL **: g_object_class_install_property: assertion 'class->set_property != NULL' failed (seahorse:20931): GLib-GObject-CRITICAL **: Object class SeahorseUnknownSource doesn't implement property 'label' from interface 'SeahorsePlace' (seahorse:20931): GLib-GObject-CRITICAL **: Flags for property 'label' on class 'SeahorseServerSource' remove functionality compared with the property on interface 'SeahorsePlace' (seahorse:20931): GLib-GObject-CRITICAL **: g_value_type_transformable: assertion 'G_TYPE_IS_VALUE (src_type)' failed (seahorse:20931): Gtk-WARNING **: /build/gtk+3.0-2Ut_nl/gtk+3.0-3.18.9/./gtk/gtkliststore.c:836: Unable to convert from (null) to gchararray Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. ** (seahorse:20931): CRITICAL **: egg_datetime_set_clamp_date: assertion 'minyear <= maxyear' failed Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. (seahorse:20931): GLib-GObject-CRITICAL **: g_value_type_transformable: assertion 'G_TYPE_IS_VALUE (src_type)' failed (seahorse:20931): Gtk-WARNING **: /build/gtk+3.0-2Ut_nl/gtk+3.0-3.18.9/./gtk/gtkliststore.c:836: Unable to convert from (null) to gchararray Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. ** (seahorse:20931): CRITICAL **: egg_datetime_set_clamp_date: assertion 'minyear <= maxyear' failed Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. (seahorse:20931): GLib-GObject-CRITICAL **: g_value_type_transformable: assertion 'G_TYPE_IS_VALUE (src_type)' failed (seahorse:20931): Gtk-WARNING **: /build/gtk+3.0-2Ut_nl/gtk+3.0-3.18.9/./gtk/gtkliststore.c:836: Unable to convert from (null) to gchararray Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. ** (seahorse:20931): CRITICAL **: egg_datetime_set_clamp_date: assertion 'minyear <= maxyear' failed Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged. Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged.
Created attachment 357670 [details] Bug without gobject_class->set_property = seahorse_gpgme_keyring_set_property I confirm, with set_property not initialized, I can't view the gpg keys gobject_class = G_OBJECT_CLASS (klass); gobject_class->get_property = seahorse_gpgme_keyring_get_property; //gobject_class->set_property = seahorse_gpgme_keyring_set_property; gobject_class->dispose = seahorse_gpgme_keyring_dispose; gobject_class->finalize = seahorse_gpgme_keyring_finalize;
Created attachment 358102 [details] [review] "Enhanced" management of ssh-key's algo types depends on patch 357647
Review of attachment 357647 [details] [review]: Please remove some trailing whitespace introductions and the *_set_property workarounds. ::: ssh/seahorse-ssh-key-data.c @@ +49,3 @@ case SSH_ALGO_RSA: /* Seems accurate to nearest 8 bits */ + return ((len - 23) * 8); What is this (unrelated) change about?
Correcting value displayed in Properties/Details/Strength of a rsa key See https://bugzilla.gnome.org/show_bug.cgi?id=754028#c4
Created attachment 359270 [details] [review] Management of ssh keys of type ecdsa and ed25519 Added comment about rsa key in function calc_bits see https://bugzilla.gnome.org/show_bug.cgi?id=754028#c20
Downstream issue id: https://bugzilla.redhat.com/show_bug.cgi?id=1435924
Support for both ECDSA and ED25519 added as of commit 1fa7434.