openssh unix dev - Apr 2020 - [PATCH 3/3] Keep rounds when changing passphrase and comment in private key file

?Keep rounds when changing passphrase and comment in private key file

This patch fixes the keygen-change regression test.


---
?ssh-keygen.c | 18 ++++++++++++++----
?1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 6dd17c48be5e..a848edc33b5d 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1425,6 +1425,7 @@ do_change_passphrase(struct passwd *pw)
???? char *old_passphrase, *passphrase1, *passphrase2;
???? struct stat st;
???? struct sshkey *private;
+??? struct sshkey_vault *vault_info;
???? int r;
?
???? if (!have_identity)
@@ -1432,7 +1433,7 @@ do_change_passphrase(struct passwd *pw)
???? if (stat(identity_file, &st) == -1)
???? ??? fatal("%s: %s", identity_file, strerror(errno));
???? /* Try to load the file with empty passphrase. */
-??? r = sshkey_load_private(identity_file, "", &private,
&comment, NULL);
+??? r = sshkey_load_private(identity_file, "", &private,
&comment,
&vault_info);
???? if (r == SSH_ERR_KEY_WRONG_PASSPHRASE) {
???? ??? if (identity_passphrase)
???? ??? ??? old_passphrase = xstrdup(identity_passphrase);
@@ -1441,7 +1442,7 @@ do_change_passphrase(struct passwd *pw)
???? ??? ??? ??? read_passphrase("Enter old passphrase: ",
???? ??? ??? ??? RP_ALLOW_STDIN);
???? ??? r = sshkey_load_private(identity_file, old_passphrase,
-??? ??? ??? &private, &comment, NULL);
+??? ??? ??? &private, &comment, &vault_info);
???? ??? freezero(old_passphrase, strlen(old_passphrase));
???? ??? if (r != 0)
???? ??? ??? goto badkey;
@@ -1476,6 +1477,10 @@ do_change_passphrase(struct passwd *pw)
???? ??? freezero(passphrase2, strlen(passphrase2));
???? }
?
+??? if (vault_info != NULL && vault_info->kdfname != NULL &&
strcmp(vault_info->kdfname, "bcrypt") == 0 && rounds == 0)
{
+??? ??? rounds = vault_info->rounds;
+??? ??? printf("Keeping existing rounds %d\n", rounds);
+??? }
???? /* Save the file using the new passphrase. */
???? if ((r = sshkey_save_private(private, identity_file, passphrase1,
???? ??? comment, private_key_format, openssh_format_cipher, rounds)) !0) {
@@ -1532,6 +1537,7 @@ do_change_comment(struct passwd *pw, const char
*identity_comment)
???? char new_comment[1024], *comment, *passphrase;
???? struct sshkey *private;
???? struct sshkey *public;
+??? struct sshkey_vault *vault_info;
???? struct stat st;
???? FILE *f;
???? int r, fd;
@@ -1541,7 +1547,7 @@ do_change_comment(struct passwd *pw, const char
*identity_comment)
???? if (stat(identity_file, &st) == -1)
???? ??? fatal("%s: %s", identity_file, strerror(errno));
???? if ((r = sshkey_load_private(identity_file, "",
-??? ??? &private, &comment, NULL)) == 0)
+??? ??? &private, &comment, &vault_info)) == 0)
???? ??? passphrase = xstrdup("");
???? else if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
???? ??? fatal("Cannot load private key \"%s\": %s.",
@@ -1556,7 +1562,7 @@ do_change_comment(struct passwd *pw, const char
*identity_comment)
???? ??? ??? ??? RP_ALLOW_STDIN);
???? ??? /* Try to load using the passphrase. */
???? ??? if ((r = sshkey_load_private(identity_file, passphrase,
-??? ??? ??? &private, &comment, NULL)) != 0) {
+??? ??? ??? &private, &comment, &vault_info)) != 0) {
???? ??? ??? freezero(passphrase, strlen(passphrase));
???? ??? ??? fatal("Cannot load private key \"%s\": %s.",
???? ??? ??? ??? identity_file, ssh_err(r));
@@ -1596,6 +1602,10 @@ do_change_comment(struct passwd *pw, const char
*identity_comment)
???? ??? exit(0);
???? }
?
+??? if (vault_info != NULL && vault_info->kdfname != NULL &&
strcmp(vault_info->kdfname, "bcrypt") == 0 && rounds == 0)
{
+??? ??? rounds = vault_info->rounds;
+??? ??? printf("Keeping existing rounds %d\n", rounds);
+??? }
???? /* Save the file using the new passphrase. */
???? if ((r = sshkey_save_private(private, identity_file, passphrase,
???? ??? new_comment, private_key_format, openssh_format_cipher,
-- 
2.17.1

Hi,

Any feedback on this patch series ?

We have the possibility with ssh-keygen to chose the cipher and the
round number of the bcrypt key derivation function, but we don't have
the possibility to examine the chosen values for a given private key,
which is strange.

It is good to be able to chose the number of rounds and the cipher,
because each user can adapt the value depending on its security target,
but it is very dangerous to reset both cipher and round to the default
values when -c or -p are used on the private without giving again the -Z
and -a option with original values. Values that you cannot recover
because there is no easy way to display them without an external tool.

Thank for your feedback.

Best regards

Lo?c

On 25/04/2020 at 03:01, Lo?c wrote :
> ?Keep rounds when changing passphrase and comment in private key file
>
> This patch fixes the keygen-change regression test.
>
>
> ---
> ?ssh-keygen.c | 18 ++++++++++++++----
> ?1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/ssh-keygen.c b/ssh-keygen.c
> index 6dd17c48be5e..a848edc33b5d 100644
> --- a/ssh-keygen.c
> +++ b/ssh-keygen.c
> @@ -1425,6 +1425,7 @@ do_change_passphrase(struct passwd *pw)
> ???? char *old_passphrase, *passphrase1, *passphrase2;
> ???? struct stat st;
> ???? struct sshkey *private;
> +??? struct sshkey_vault *vault_info;
> ???? int r;
> ?
> ???? if (!have_identity)
> @@ -1432,7 +1433,7 @@ do_change_passphrase(struct passwd *pw)
> ???? if (stat(identity_file, &st) == -1)
> ???? ??? fatal("%s: %s", identity_file, strerror(errno));
> ???? /* Try to load the file with empty passphrase. */
> -??? r = sshkey_load_private(identity_file, "", &private,
&comment, NULL);
> +??? r = sshkey_load_private(identity_file, "", &private,
&comment,
> &vault_info);
> ???? if (r == SSH_ERR_KEY_WRONG_PASSPHRASE) {
> ???? ??? if (identity_passphrase)
> ???? ??? ??? old_passphrase = xstrdup(identity_passphrase);
> @@ -1441,7 +1442,7 @@ do_change_passphrase(struct passwd *pw)
> ???? ??? ??? ??? read_passphrase("Enter old passphrase: ",
> ???? ??? ??? ??? RP_ALLOW_STDIN);
> ???? ??? r = sshkey_load_private(identity_file, old_passphrase,
> -??? ??? ??? &private, &comment, NULL);
> +??? ??? ??? &private, &comment, &vault_info);
> ???? ??? freezero(old_passphrase, strlen(old_passphrase));
> ???? ??? if (r != 0)
> ???? ??? ??? goto badkey;
> @@ -1476,6 +1477,10 @@ do_change_passphrase(struct passwd *pw)
> ???? ??? freezero(passphrase2, strlen(passphrase2));
> ???? }
> ?
> +??? if (vault_info != NULL && vault_info->kdfname != NULL
&&
> strcmp(vault_info->kdfname, "bcrypt") == 0 && rounds
== 0) {
> +??? ??? rounds = vault_info->rounds;
> +??? ??? printf("Keeping existing rounds %d\n", rounds);
> +??? }
> ???? /* Save the file using the new passphrase. */
> ???? if ((r = sshkey_save_private(private, identity_file, passphrase1,
> ???? ??? comment, private_key_format, openssh_format_cipher, rounds)) !>
0) {
> @@ -1532,6 +1537,7 @@ do_change_comment(struct passwd *pw, const char
> *identity_comment)
> ???? char new_comment[1024], *comment, *passphrase;
> ???? struct sshkey *private;
> ???? struct sshkey *public;
> +??? struct sshkey_vault *vault_info;
> ???? struct stat st;
> ???? FILE *f;
> ???? int r, fd;
> @@ -1541,7 +1547,7 @@ do_change_comment(struct passwd *pw, const char
> *identity_comment)
> ???? if (stat(identity_file, &st) == -1)
> ???? ??? fatal("%s: %s", identity_file, strerror(errno));
> ???? if ((r = sshkey_load_private(identity_file, "",
> -??? ??? &private, &comment, NULL)) == 0)
> +??? ??? &private, &comment, &vault_info)) == 0)
> ???? ??? passphrase = xstrdup("");
> ???? else if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
> ???? ??? fatal("Cannot load private key \"%s\": %s.",
@@ -1556,7 +1562,7 @@ do_change_comment(struct passwd *pw, const
> char *identity_comment) ???? ??? ??? ??? RP_ALLOW_STDIN); ???? ??? /*
> Try to load using the passphrase. */ ???? ??? if ((r >
sshkey_load_private(identity_file, passphrase, -??? ??? ??? &private,
> &comment, NULL)) != 0) { +??? ??? ??? &private, &comment,
> &vault_info)) != 0) { ???? ??? ??? freezero(passphrase,
> strlen(passphrase)); ???? ??? ??? fatal("Cannot load private key
\"%s\": %s.",
> ???? ??? ??? ??? identity_file, ssh_err(r));
> @@ -1596,6 +1602,10 @@ do_change_comment(struct passwd *pw, const char
> *identity_comment)
> ???? ??? exit(0);
> ???? }
> ?
> +??? if (vault_info != NULL && vault_info->kdfname != NULL
&&
> strcmp(vault_info->kdfname, "bcrypt") == 0 && rounds
== 0) {
> +??? ??? rounds = vault_info->rounds;
> +??? ??? printf("Keeping existing rounds %d\n", rounds);
> +??? }
> ???? /* Save the file using the new passphrase. */
> ???? if ((r = sshkey_save_private(private, identity_file, passphrase,
> ???? ??? new_comment, private_key_format, openssh_format_cipher,
> --
> 2.17.1
>
>
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev