[
https://issues.apache.org/jira/browse/SSHD-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16718592#comment-16718592
]
Thomas Wolf commented on SSHD-708:
----------------------------------
This would have need a review.
* Thanks for the attribution, but it wouldn't have been necessary. Most of the
code I had written is not in that commit anyway but in the parent commit.
* It would have helped if you had created your own PR for this. Now it's
rather difficult for me to comment on this. :(
* It's *broken*.
[MAX_ROUNDS|https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java#L58]
is wrong. OpenSSH does *not* store an exponent for the rounds but the number
of rounds. I even had a comment in my code pointing this out. The code as is
will fail to decode a key generated with {{ssh-keygen -t ed25519 -a 33}}.
Besides, I don't understand the comment about "doubling". What doubles?
* I very much like the addition of tests for the password-provider re-try loop.
* Setting local variables to {{null}} at the end of methods doesn't improve
"getting rid of sensitive data", does it? For arrays, yes, but if a password
comes in as a String, there's not much one can do.
*
[bcryptKdf|https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java#L192]
doesn't throw any checked exceptions. The [catch block you added
above|https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java#L167]
should actually be inside {{bcryptKdf}}, and it should not handle
{{IOException}} (never thrown there), only {{GeneralSecurityException}}. This
should simply be as in my PR.
* Otherwise it's very hard to review this because the code has changed very
much, has become IMO needlessly general and complicated, and the individual
commits include unrelated formatting changes.
> Add support for password encrypted OpenSSH private key files
> ------------------------------------------------------------
>
> Key: SSHD-708
> URL: https://issues.apache.org/jira/browse/SSHD-708
> Project: MINA SSHD
> Issue Type: Improvement
> Affects Versions: 1.4.0
> Reporter: Goldstein Lyor
> Assignee: Goldstein Lyor
> Priority: Minor
> Fix For: 2.1.1
>
>
> The current code supports only reading un-encrypted private key files
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
