soatok/minisign-php

Public key is not deserialized correctly

vuryss opened this issue · 2 comments

The public key file is not deserialized correctly - it takes first 8 bytes as keyId and next 32 as the key itself. But the PK actually starts with 2 bytes containing the algorithm used. So the ID and the PK itself are offset by 2 characters.

For example using the public key in your tests:

Public key (hex encoded):
45644535c704df22f46c6af7f8916cd65e0f3a45e123b0830a9352de2fdc0f20b0f3bd2e4b724112ad69

It has 84 characters, which are 42 in binary.

  • First 4 (4564) correspond to (Ed), which is the algorithm
  • Next 16 (4535c704df22f46c) correspond to (6CF422DF04C73545) - the key ID
  • Next 64 (6af7f8916cd65e0f3a45e123b0830a9352de2fdc0f20b0f3bd2e4b724112ad69) correspond to the public key itself.

Currently tho, we starting to substring from the start for the Key ID and right after for the PK. That results in KeyID containing the algorithm and the public key containing parts of the KeyID. Basically everything is offset by 2 characters.

You can see that here:

$keyId = Binary::safeSubstr($decoded, 0, 8);

The fix is really simple - just take the key ID from '2' to '10' and the PK from '10' to '42'
Which makes the correct lines:

$algo = Binary::safeSubstr($decoded, 0, 2); // Ignored, here for info what first 2 characters are
$keyId = Binary::safeSubstr($decoded, 2, 8);
$pk = Binary::safeSubstr($decoded, 10, 32);

Here is the documentation of the public key format:
https://jedisct1.github.io/minisign/#public-key-format

But the PK actually starts with 2 bytes containing the algorithm used.

In minisign, yes. In this lib, no, as can be seen there:

Base64::encodeUnpadded($this->keyId . $this->pk);

It should be:

 Base64::encodeUnpadded($this->signatureAlgorithm . $this->keyId . $this->pk);

Where $this->signatureAlgorithm would be 'Ed'. Then it produces valid public keys that minisign binary can process.

This will be fixed in v0.6.0