[ 
https://issues.apache.org/jira/browse/SSHD-997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17112365#comment-17112365
 ] 

Thomas Wolf edited comment on SSHD-997 at 5/20/20, 3:44 PM:
------------------------------------------------------------

I think I know why it is all zeroes for a key read from a file.

Check {{OpenSSHEd25519PrivateKeyEntryDecoder}}, lines 103ff:

{code:java}
            byte[] sk = Arrays.copyOf(keypair, SK_SIZE);
            EdDSAPrivateKey privateKey;
            try {
                // create the private key
                EdDSAParameterSpec params = 
EdDSANamedCurveTable.getByName(EdDSASecurityProviderUtils.CURVE_ED25519_SHA512);
                privateKey = generatePrivateKey(new EdDSAPrivateKeySpec(sk, 
params));
            } finally {
                // get rid of sensitive data a.s.a.p
                Arrays.fill(sk, (byte) 0);
            }
{code}

sshd clears the SK read from the file. But that SK is the same object as 
{{privateKey.seed}}; it is _not_ copied during generatePrivateKey.

The key still works with sshd (I use ed25519 keys with sshd in JGit daily), but 
this  breaks round-tripping: reading a key from a file and writing it again 
will produce a different file. (And of course it also breaks 
{{KeyUtils.compareKeys()}}.)

The fix in sshd is to remove the {{try-finally}} and the {{Arrays.fill(sk, 
(byte) 0);}}. A fix in i2p would be to make sure that the private key generated 
from a spec does not reference any arrays passed in externally, but to copy the 
data such that the key is the sole owner.

So this is actually not a "minor feature" but really a "bug".


was (Author: wolft):
I think I know why it is all zeroes for a key read from a file.

Check {{OpenSSHEd25519PrivateKeyEntryDecoder}}, lines 103ff:

{code:java}
            byte[] sk = Arrays.copyOf(keypair, SK_SIZE);
            EdDSAPrivateKey privateKey;
            try {
                // create the private key
                EdDSAParameterSpec params = 
EdDSANamedCurveTable.getByName(EdDSASecurityProviderUtils.CURVE_ED25519_SHA512);
                privateKey = generatePrivateKey(new EdDSAPrivateKeySpec(sk, 
params));
            } finally {
                // get rid of sensitive data a.s.a.p
                Arrays.fill(sk, (byte) 0);
            }
{code}

sshd clears the SK read from the file. But that SK is the same object as 
{{privateKey.seed}}; it is _not_ copied during generatePrivateKey.

The key still works with sshd (I use ed25519 keys with sshd in JGit daily), but 
this  breaks round-tripping: reading a key from a file and writing it again 
will produce a different file. (And of course it also breaks 
{{KeyUtils.compareKeys()}}.)

The fix in sshd is to remove the {{try-finally}} and the {{Arrays.fill(sk, 
(byte) 0);}}. A fix in i2p would be to make sure that the private key generated 
from a spec does not reference any arrays passed in externally, but to copy the 
data such that the key is the sole owner.

So this is actually not a "minor feature" but actually a "bug".

> Replace EdDSA-Java library with new ed25519-elisabeth implementation
> --------------------------------------------------------------------
>
>                 Key: SSHD-997
>                 URL: https://issues.apache.org/jira/browse/SSHD-997
>             Project: MINA SSHD
>          Issue Type: New Feature
>    Affects Versions: 2.4.0
>            Reporter: David Ostrovsky
>            Priority: Minor
>
> Recent addition to the SSHD library revealed issues with seed attribute in 
> EdDSA-Java library:
> {code:java}
> +    private boolean compare(KeyPair a, KeyPair b) {
> +        if ("EDDSA".equals(data.algorithm)) {
> +            // Bug in net.i2p.crypto.eddsa and in sshd? Both also compare the
> +            // seed of the private key, but for a generated key, this is some
> +            // random value, while it is all zeroes for a key read from a 
> file.
> +            return KeyUtils.compareKeys(a.getPublic(), b.getPublic())
> +                    && Objects.equals(((EdDSAKey) 
> a.getPrivate()).getParams(),
> +                            ((EdDSAKey) b.getPrivate()).getParams());
> +        }
> {code}
> The corresponding issue: [1] upstream pointing to the new library: 
> [1] https://github.com/str4d/ed25519-java/issues/30#issuecomment-573389252
> [2] https://github.com/cryptography-cafe/ed25519-elisabeth



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Reply via email to