generateKeyPairSync('ed25519') + parseKey() fails ~0.4% of the time with "Malformed OpenSSH public key" (over-stripping leading zero bytes in keygen.js)

[Soulstears2539]

Environment

• ssh2 version: 1.17.0 (latest published at time of writing)
• Node.js version: v24.14.0
• OS: Windows 10

Summary

utils.generateKeyPairSync('ed25519') occasionally produces a public key that
utils.parseKey() then rejects as Malformed OpenSSH public key. This is
reproducible with plain synchronous calls, no network/SSH connection involved:

const { utils } = require('ssh2');

for (let i = 0; i < 20000; i++) {
  const kp = utils.generateKeyPairSync('ed25519');
  const parsed = utils.parseKey(kp.public);
  if (parsed instanceof Error) {
    console.log('FAILURE at', i, parsed.message);
  }
}

In a 20000-iteration run this failed 85 times (0.425%) -- close to the
theoretically expected 1/256 (~0.39%) rate explained below.

Root cause

In lib/keygen.js, parseDERs() handles the ed25519 case by reading the
DER-encoded SubjectPublicKeyInfo BIT STRING and then "cleaning" it like this:

let pubBin = reader.readString(Ber.BitString, true);
{
  // Remove leading zero bytes
  let i = 0;
  for (; i < pubBin.length && pubBin[i] === 0x00; ++i);
  if (i > 0)
    pubBin = pubBin.slice(i);
}

For an ed25519 SPKI, the BIT STRING content is always exactly 33 bytes:
1 mandatory ASN.1 "unused bits" count byte (always 0x00, since the key is
byte-aligned) followed by the raw 32-byte public key. The correct operation is
therefore to strip exactly that one byte, unconditionally.

Instead, the code loops over all leading zero bytes. Whenever the raw
32-byte public key itself also happens to start with one or more 0x00
bytes (~1/256 chance per key, i.e. whenever pubBin[1] === 0x00), the loop
over-strips real key material, producing a corrupted/truncated key that is
too short. parseKey() (via the OpenSSH-format public key length checks)
then correctly rejects this corrupted data as malformed -- the bug is in the
DER decoder producing the wrong bytes in the first place, not in the
validation that catches it afterward.

Fix

Strip exactly one byte when it is 0x00, not a variable-length run:

       let pubBin = reader.readString(Ber.BitString, true);
       {
-        // Remove leading zero bytes
-        let i = 0;
-        for (; i < pubBin.length && pubBin[i] === 0x00; ++i);
-        if (i > 0)
-          pubBin = pubBin.slice(i);
+        // The BIT STRING content is exactly one "unused bits" count byte
+        // (always 0x00 here, since an ED25519 key is byte-aligned) followed
+        // by the raw 32-byte public key. Strip exactly that one byte --
+        // NOT a variable-length run of zero bytes.
+        if (pubBin.length > 0 && pubBin[0] === 0x00)
+          pubBin = pubBin.slice(1);
       }

After applying this fix, the same 20000-iteration-style loop (run for 3000
iterations post-fix) produced 0 failures, consistent with the
theoretical expectation that the bug is now fully eliminated (not just less
likely).

Impact

Any code path that generates an ed25519 key pair with generateKeyPairSync
and immediately parses/uses the resulting public key (e.g. test fixtures
that spin up an in-process ssh2.Server, or any consumer validating its own
freshly generated key) has a ~0.4% chance per key of hitting a hard failure.
This is a classic source of flaky tests/CI failures that are very hard to
track down without instrumenting parseDERs() directly, since the failure
is silent and looks like a generic "malformed key" input error rather than
a bug in key generation itself.

Note: the analogous ec (ECDSA) case in the same function does not have
this bug -- it slices off a fixed leading 0x04 marker byte via a direct
index/length check, not a variable-length zero-stripping loop, so it is not
affected.

I'm happy to open a PR with this fix if useful.