Sort out the mess with OpenSSH key file formats.

When I implemented reading and writing of the new format a couple of
weeks ago, I kept them strictly separate in the UI, so you have to ask
for the format you want when exporting. But in fact this is silly,
because not every key type can be saved in both formats, and OpenSSH
itself has the policy of using the old format for key types it can
handle, unless specifically asked to use the new one.

So I've now arranged that the key file format enum has three values
for OpenSSH: PEM, NEW and AUTO. Files being loaded are identified as
either PEM or NEW, which describe the two physical file formats. But
exporting UIs present either AUTO or NEW, where AUTO is the virtual
format meaning 'save in the old format if possible, otherwise the new
one'.

diff --git a/cmdgen.c b/cmdgen.c
index 73383163063b2c52f1ebf9d3d688a304f2dcb41e..7809c89186b489ecfa30f549e3888b62fb273774 100644 (file)
--- a/cmdgen.c
+++ b/cmdgen.c
@@ -160,10 +160,8 @@ void help(void)
            "  -O    specify output type:\n"
            "           private             output PuTTY private key format\n"
            "           private-openssh     export OpenSSH private key\n"
-           "           private-openssh-pem export OpenSSH private key "
-                                             "(force old PEM format)\n"
            "           private-openssh-new export OpenSSH private key "
-                                             "(force new format)\n"
+                                             "(force new file format)\n"
            "           private-sshcom      export ssh.com private key\n"
            "           public              standard / ssh.com public key\n"
            "           public-openssh      OpenSSH public key\n"
@@ -271,7 +269,7 @@ int main(int argc, char **argv)
     Filename *infilename = NULL, *outfilename = NULL;
     enum { NOKEYGEN, RSA1, RSA2, DSA, ECDSA, ED25519 } keytype = NOKEYGEN;
     char *outfile = NULL, *outfiletmp = NULL;
-    enum { PRIVATE, PUBLIC, PUBLICO, FP, OPENSSH_PEM,
+    enum { PRIVATE, PUBLIC, PUBLICO, FP, OPENSSH_AUTO,
            OPENSSH_NEW, SSHCOM } outtype = PRIVATE;
     int bits = -1;
     char *comment = NULL, *origcomment = NULL;
@@ -467,9 +465,8 @@ int main(int argc, char **argv)
                            outtype = PRIVATE;
                        else if (!strcmp(p, "fingerprint"))
                            outtype = FP;
-                       else if (!strcmp(p, "private-openssh") ||
-                                 !strcmp(p, "private-openssh-pem"))
-                           outtype = OPENSSH_PEM, sshver = 2;
+                       else if (!strcmp(p, "private-openssh"))
+                           outtype = OPENSSH_AUTO, sshver = 2;
                        else if (!strcmp(p, "private-openssh-new"))
                            outtype = OPENSSH_NEW, sshver = 2;
                        else if (!strcmp(p, "private-sshcom"))
@@ -569,7 +566,7 @@ int main(int argc, char **argv)
      * We must save the private part when generating a new key.
      */
     if (keytype != NOKEYGEN &&
-       (outtype != PRIVATE && outtype != OPENSSH_PEM &&
+       (outtype != PRIVATE && outtype != OPENSSH_AUTO &&
          outtype != OPENSSH_NEW && outtype != SSHCOM)) {
        fprintf(stderr, "puttygen: this would generate a new key but "
                "discard the private part\n");
@@ -634,6 +631,10 @@ int main(int argc, char **argv)
            }
            sshver = 2;
            break;
+
+         case SSH_KEYTYPE_OPENSSH_AUTO:
+          default:
+            assert(0 && "Should never see these types on an input file");
        }
     }
 
@@ -649,7 +650,7 @@ int main(int argc, char **argv)
      */
     if ((intype == SSH_KEYTYPE_SSH1 && outtype == PRIVATE) ||
        (intype == SSH_KEYTYPE_SSH2 && outtype == PRIVATE) ||
-       (intype == SSH_KEYTYPE_OPENSSH_PEM && outtype == OPENSSH_PEM) ||
+       (intype == SSH_KEYTYPE_OPENSSH_PEM && outtype == OPENSSH_AUTO) ||
        (intype == SSH_KEYTYPE_OPENSSH_NEW && outtype == OPENSSH_NEW) ||
        (intype == SSH_KEYTYPE_SSHCOM && outtype == SSHCOM)) {
        if (!outfile) {
@@ -668,7 +669,7 @@ int main(int argc, char **argv)
             * Bomb out rather than automatically choosing to write
             * a private key file to stdout.
             */
-           if (outtype == PRIVATE || outtype == OPENSSH_PEM ||
+           if (outtype == PRIVATE || outtype == OPENSSH_AUTO ||
                 outtype == OPENSSH_NEW || outtype == SSHCOM) {
                fprintf(stderr, "puttygen: need to specify an output file\n");
                return 1;
@@ -682,7 +683,7 @@ int main(int argc, char **argv)
      * out a private key format, or (b) the entire input key file
      * is encrypted.
      */
-    if (outtype == PRIVATE || outtype == OPENSSH_PEM ||
+    if (outtype == PRIVATE || outtype == OPENSSH_AUTO ||
         outtype == OPENSSH_NEW || outtype == SSHCOM ||
        intype == SSH_KEYTYPE_OPENSSH_PEM ||
        intype == SSH_KEYTYPE_OPENSSH_NEW ||
@@ -1102,7 +1103,7 @@ int main(int argc, char **argv)
        }
        break;
        
-      case OPENSSH_PEM:
+      case OPENSSH_AUTO:
       case OPENSSH_NEW:
       case SSHCOM:
        assert(sshver == 2);
@@ -1110,8 +1111,8 @@ int main(int argc, char **argv)
        random_ref(); /* both foreign key types require randomness,
                        * for IV or padding */
         switch (outtype) {
-          case OPENSSH_PEM:
-            real_outtype = SSH_KEYTYPE_OPENSSH_PEM;
+          case OPENSSH_AUTO:
+            real_outtype = SSH_KEYTYPE_OPENSSH_AUTO;
             break;
           case OPENSSH_NEW:
             real_outtype = SSH_KEYTYPE_OPENSSH_NEW;

diff --git a/import.c b/import.c
index 5481ce8f141a80c5d12248832381261579f745eb..34f29acf3600007a9a2149645e78ac1c8c041796 100644 (file)
--- a/import.c
+++ b/import.c
@@ -20,6 +20,8 @@ struct ssh2_userkey *openssh_pem_read(const Filename *filename,
 struct ssh2_userkey *openssh_new_read(const Filename *filename,
                                       char *passphrase,
                                       const char **errmsg_p);
+int openssh_auto_write(const Filename *filename, struct ssh2_userkey *key,
+                       char *passphrase);
 int openssh_pem_write(const Filename *filename, struct ssh2_userkey *key,
                       char *passphrase);
 int openssh_new_write(const Filename *filename, struct ssh2_userkey *key,
@@ -117,8 +119,8 @@ int export_ssh1(const Filename *filename, int type, struct RSAKey *key,
 int export_ssh2(const Filename *filename, int type,
                 struct ssh2_userkey *key, char *passphrase)
 {
-    if (type == SSH_KEYTYPE_OPENSSH_PEM)
-       return openssh_pem_write(filename, key, passphrase);
+    if (type == SSH_KEYTYPE_OPENSSH_AUTO)
+       return openssh_auto_write(filename, key, passphrase);
     if (type == SSH_KEYTYPE_OPENSSH_NEW)
        return openssh_new_write(filename, key, passphrase);
     if (type == SSH_KEYTYPE_SSHCOM)
@@ -1888,6 +1890,28 @@ int openssh_new_write(const Filename *filename, struct ssh2_userkey *key,
     return ret;
 }
 
+/* ----------------------------------------------------------------------
+ * The switch function openssh_auto_write(), which chooses one of the
+ * concrete OpenSSH output formats based on the key type.
+ */
+int openssh_auto_write(const Filename *filename, struct ssh2_userkey *key,
+                       char *passphrase)
+{
+    /*
+     * The old OpenSSH format supports a fixed list of key types. We
+     * assume that anything not in that fixed list is newer, and hence
+     * will use the new format.
+     */
+    if (key->alg == &ssh_dss ||
+        key->alg == &ssh_rsa ||
+        key->alg == &ssh_ecdsa_nistp256 ||
+        key->alg == &ssh_ecdsa_nistp384 ||
+        key->alg == &ssh_ecdsa_nistp521)
+        return openssh_pem_write(filename, key, passphrase);
+    else
+        return openssh_new_write(filename, key, passphrase);
+}
+
 /* ----------------------------------------------------------------------
  * Code to read ssh.com private keys.
  */

diff --git a/ssh.h b/ssh.h
index bb2668473ec96466172c3d8d15175840a5726d73..83cceb1723799e7aae501f30112210f661183237 100644 (file)
--- a/ssh.h
+++ b/ssh.h
@@ -674,6 +674,34 @@ enum {
     SSH_KEYTYPE_UNOPENABLE,
     SSH_KEYTYPE_UNKNOWN,
     SSH_KEYTYPE_SSH1, SSH_KEYTYPE_SSH2,
+    /*
+     * The OpenSSH key types deserve a little explanation. OpenSSH has
+     * two physical formats for private key storage: an old PEM-based
+     * one largely dictated by their use of OpenSSL and full of ASN.1,
+     * and a new one using the same private key formats used over the
+     * wire for talking to ssh-agent. The old format can only support
+     * a subset of the key types, because it needs redesign for each
+     * key type, and after a while they decided to move to the new
+     * format so as not to have to do that.
+     *
+     * On input, key files are identified as either
+     * SSH_KEYTYPE_OPENSSH_PEM or SSH_KEYTYPE_OPENSSH_NEW, describing
+     * accurately which actual format the keys are stored in.
+     *
+     * On output, however, we default to following OpenSSH's own
+     * policy of writing out PEM-style keys for maximum backwards
+     * compatibility if the key type supports it, and otherwise
+     * switching to the new format. So the formats you can select for
+     * output are SSH_KEYTYPE_OPENSSH_NEW (forcing the new format for
+     * any key type), and SSH_KEYTYPE_OPENSSH_AUTO to use the oldest
+     * format supported by whatever key type you're writing out.
+     *
+     * So we have three type codes, but only two of them usable in any
+     * given circumstance. An input key file will never be identified
+     * as AUTO, only PEM or NEW; key export UIs should not be able to
+     * select PEM, only AUTO or NEW.
+     */
+    SSH_KEYTYPE_OPENSSH_AUTO,
     SSH_KEYTYPE_OPENSSH_PEM,
     SSH_KEYTYPE_OPENSSH_NEW,
     SSH_KEYTYPE_SSHCOM

diff --git a/sshpubk.c b/sshpubk.c
index 53793f0dea69188623af45784dcdb8a67827b23f..6aba82fc80630d19f2cac5bbfbf296c7d8d6752d 100644 (file)
--- a/sshpubk.c
+++ b/sshpubk.c
@@ -1204,6 +1204,13 @@ char *key_type_to_str(int type)
       case SSH_KEYTYPE_OPENSSH_PEM: return "OpenSSH SSH-2 private key (old PEM format)"; break;
       case SSH_KEYTYPE_OPENSSH_NEW: return "OpenSSH SSH-2 private key (new format)"; break;
       case SSH_KEYTYPE_SSHCOM: return "ssh.com SSH-2 private key"; break;
+        /*
+         * This function is called with a key type derived from
+         * looking at an actual key file, so the output-only type
+         * OPENSSH_AUTO should never get here, and is much an INTERNAL
+         * ERROR as a code we don't even understand.
+         */
+      case SSH_KEYTYPE_OPENSSH_AUTO: return "INTERNAL ERROR (OPENSSH_AUTO)"; break;
       default: return "INTERNAL ERROR"; break;
     }
 }

diff --git a/windows/winpgen.c b/windows/winpgen.c
index 858b4581e11a0013009a07a0411470062f9c754c..093479ff77969b55464dc42872def2f2af4772f1 100644 (file)
--- a/windows/winpgen.c
+++ b/windows/winpgen.c
@@ -537,7 +537,7 @@ enum {
     IDC_ABOUT,
     IDC_GIVEHELP,
     IDC_IMPORT,
-    IDC_EXPORT_OPENSSH_PEM, IDC_EXPORT_OPENSSH_NEW,
+    IDC_EXPORT_OPENSSH_AUTO, IDC_EXPORT_OPENSSH_NEW,
     IDC_EXPORT_SSHCOM
 };
 
@@ -586,7 +586,7 @@ void ui_set_state(HWND hwnd, struct MainDlgState *state, int status)
         EnableMenuItem(state->keymenu, IDC_KEYSSH2ED25519,
                        MF_ENABLED|MF_BYCOMMAND);
        EnableMenuItem(state->cvtmenu, IDC_IMPORT, MF_ENABLED|MF_BYCOMMAND);
-       EnableMenuItem(state->cvtmenu, IDC_EXPORT_OPENSSH_PEM,
+       EnableMenuItem(state->cvtmenu, IDC_EXPORT_OPENSSH_AUTO,
                       MF_GRAYED|MF_BYCOMMAND);
        EnableMenuItem(state->cvtmenu, IDC_EXPORT_OPENSSH_NEW,
                       MF_GRAYED|MF_BYCOMMAND);
@@ -619,7 +619,7 @@ void ui_set_state(HWND hwnd, struct MainDlgState *state, int status)
         EnableMenuItem(state->keymenu, IDC_KEYSSH2ED25519,
                        MF_GRAYED|MF_BYCOMMAND);
        EnableMenuItem(state->cvtmenu, IDC_IMPORT, MF_GRAYED|MF_BYCOMMAND);
-       EnableMenuItem(state->cvtmenu, IDC_EXPORT_OPENSSH_PEM,
+       EnableMenuItem(state->cvtmenu, IDC_EXPORT_OPENSSH_AUTO,
                       MF_GRAYED|MF_BYCOMMAND);
        EnableMenuItem(state->cvtmenu, IDC_EXPORT_OPENSSH_NEW,
                       MF_GRAYED|MF_BYCOMMAND);
@@ -660,7 +660,7 @@ void ui_set_state(HWND hwnd, struct MainDlgState *state, int status)
 #define do_export_menuitem(x,y) \
     EnableMenuItem(state->cvtmenu, x, MF_BYCOMMAND | \
                       (import_target_type(y)==type?MF_ENABLED:MF_GRAYED))
-       do_export_menuitem(IDC_EXPORT_OPENSSH_PEM, SSH_KEYTYPE_OPENSSH_PEM);
+       do_export_menuitem(IDC_EXPORT_OPENSSH_AUTO, SSH_KEYTYPE_OPENSSH_AUTO);
        do_export_menuitem(IDC_EXPORT_OPENSSH_NEW, SSH_KEYTYPE_OPENSSH_NEW);
        do_export_menuitem(IDC_EXPORT_SSHCOM, SSH_KEYTYPE_SSHCOM);
 #undef do_export_menuitem
@@ -902,10 +902,10 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
            menu1 = CreateMenu();
            AppendMenu(menu1, MF_ENABLED, IDC_IMPORT, "&Import key");
            AppendMenu(menu1, MF_SEPARATOR, 0, 0);
-           AppendMenu(menu1, MF_ENABLED, IDC_EXPORT_OPENSSH_PEM,
-                      "Export &OpenSSH key (old PEM format)");
+           AppendMenu(menu1, MF_ENABLED, IDC_EXPORT_OPENSSH_AUTO,
+                      "Export &OpenSSH key");
            AppendMenu(menu1, MF_ENABLED, IDC_EXPORT_OPENSSH_NEW,
-                      "Export &OpenSSH key (new format)");
+                      "Export &OpenSSH key (force new file format)");
            AppendMenu(menu1, MF_ENABLED, IDC_EXPORT_SSHCOM,
                       "Export &ssh.com key");
            AppendMenu(menu, MF_POPUP | MF_ENABLED, (UINT) menu1,
@@ -1198,7 +1198,7 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
            }
            break;
          case IDC_SAVE:
-          case IDC_EXPORT_OPENSSH_PEM:
+          case IDC_EXPORT_OPENSSH_AUTO:
           case IDC_EXPORT_OPENSSH_NEW:
           case IDC_EXPORT_SSHCOM:
            if (HIWORD(wParam) != BN_CLICKED)
@@ -1215,8 +1215,8 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
                 else
                     realtype = SSH_KEYTYPE_SSH1;
 
-                if (LOWORD(wParam) == IDC_EXPORT_OPENSSH_PEM)
-                    type = SSH_KEYTYPE_OPENSSH_PEM;
+                if (LOWORD(wParam) == IDC_EXPORT_OPENSSH_AUTO)
+                    type = SSH_KEYTYPE_OPENSSH_AUTO;
                 else if (LOWORD(wParam) == IDC_EXPORT_OPENSSH_NEW)
                     type = SSH_KEYTYPE_OPENSSH_NEW;
                 else if (LOWORD(wParam) == IDC_EXPORT_SSHCOM)
@@ -1499,7 +1499,7 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
               case IDC_BITS:
                 topic = WINHELP_CTX_puttygen_bits; break;
               case IDC_IMPORT:
-              case IDC_EXPORT_OPENSSH_PEM:
+              case IDC_EXPORT_OPENSSH_AUTO:
               case IDC_EXPORT_OPENSSH_NEW:
               case IDC_EXPORT_SSHCOM:
                 topic = WINHELP_CTX_puttygen_conversions; break;