Schaaf, Jonathan P (GE Healthcare)
2013-May-29 19:14 UTC
Patch to discourage unencrypted key generation
>>> configuration holes are the *default* configuration. ssh-keygen >>> creates passphrase frees by default if you simply hit "Enter" a few >>> times, and there is no way I've ever seen for ssh_config to reject >>> them by default when loading local keys or loading them into an >>> ssh-agent. >> >> So where are your patches? > > Excellent point. Let me see if I can unpry some tome this week to submit a patch. > But I'm concerned it will run into the "but that would change people's workflow!!!!" > world of rejected patches, even if the patch is clean.I hope I'm not submitting something while Martin is halfway through working on this, but as previously noted, the real complexities are in the change to people's workflow. Let the beatings commence. diff -ruN ssh-orig/ssh-keygen.c ssh/ssh-keygen.c --- ssh-orig/ssh-keygen.c 2013-02-10 17:32:10.000000000 -0600 +++ ssh/ssh-keygen.c 2013-05-29 13:57:50.555791814 -0500 @@ -2585,6 +2585,23 @@ printf("Passphrases do not match. Try again.\n"); goto passphrase_again; } + if (strcmp(passphrase1, "" ) == 0) { + char iknow[7]; + printf("Empty passphrases are a potential security risk. \n" ); + printf("Type \"I know\" to confirm that you want this: " ); + fflush(stdout); + + if (fgets(iknow, sizeof(iknow), stdin) == NULL) + exit(1); + if (strcmp("I know", iknow ) != 0) { + memset(passphrase1, 0, strlen(passphrase1)); + memset(passphrase2, 0, strlen(passphrase2)); + xfree(passphrase1); + xfree(passphrase2); + goto passphrase_again; + } + } + /* Clear the other copy of the passphrase. */ memset(passphrase2, 0, strlen(passphrase2)); xfree(passphrase2);
On Wed, May 29, 2013 at 3:14 PM, Schaaf, Jonathan P (GE Healthcare) <jonathan.P.schaaf at ge.com> wrote:>>>> configuration holes are the *default* configuration. ssh-keygen >>>> creates passphrase frees by default if you simply hit "Enter" a few >>>> times, and there is no way I've ever seen for ssh_config to reject >>>> them by default when loading local keys or loading them into an >>>> ssh-agent. >>> >>> So where are your patches? >> >> Excellent point. Let me see if I can unpry some tome this week to submit a patch. >> But I'm concerned it will run into the "but that would change people's workflow!!!!" >> world of rejected patches, even if the patch is clean. > > > I hope I'm not submitting something while Martin is halfway through working on this, but as previously noted, the real complexities are in the change to people's workflow. Let the beatings commence.That was me, not Martin. Adding a "you have to type the secret undocumented code phrase, without any manual page entry or documentation whatsoever" is, indeed cause for beatings with a rubber hammer. I was personally thinking "add a required '-z' command line argument for a zero-length passphrase", since the ssh-keygen is already using "-n" and "-e" for other options.
Schaaf, Jonathan P (GE Healthcare) <jonathan.P.schaaf at ge.com> wrote on Wed, 29 May 2013 at 19:14:45 +0000 in <C2DDDB22B0AE094DB5F3CE04CB9E2F2615D393 at CINURCNA02.e2k.ad.ge.com>:> I hope I'm not submitting something while Martin is halfway through > working on this, but as previously noted, the real complexities are > in the change to people's workflow. Let the beatings commence....> + printf("Empty passphrases are a potential security risk. \n" ); > + printf("Type \"I know\" to confirm that you want this: " );I don't think this is the way to go. Among other things, it precludes easy automation of this, which is bad (esp., as was noted, for host keys). Furthremore, it gives just enough information to not be helpful. WHY are they a security risk? WHERE can we find out more info? WHAT are the alternatives? --jhawk at mit.edu John Hawkinson