Attempting to Catch Invalid ed25519 Private Keys

[aidantwoods]

Opening the issue here since there isn't a specific reference to acceptable ed25519 formats in the PASETO standard, so this seems mostly relevant to the library (though arguably a format is somewhat implied by the test vector suite, which uses the 64 byte format seed || pub).

Should PASETO implementations attempt to catch malformed ed25519 private keys? This is really a user error, but lack of an early failure introduces some non ideal usability where things can be signed but can't be later verified in the event of a malformed private key, since the signer commits to a public key that isn't actually related to the seed.

Other than a length check, which is already performed, the validation I had in mind was to: feed the first 32 bytes in as the seed during import, calculate the corresponding public key, and then compare this resulting public key to the final 32 bytes of the total 64 byte imported key (i.e. the public portion of the key). If the calculated public key doesn't match, the key is invalid and so we can abort during import.

Ref: https://github.com/aidantwoods/go-paseto/issues/7

[paragonie-security]

https://github.com/paragonie/paseto/blob/master/src/Keys/AsymmetricSecretKey.php#L91

[aidantwoods]

Right 😄 what I'm getting at here is whether we want to run this check on every import?

Folks that accidentally provide an invalid input are unlikely to realise that they might want this check (if they did, they'd probably provide a valid input in the first place).

[paragonie-security]

This adds a runtime cost of a scalar multiplication every time you instantiate the object.

Instead, users who import keys should assert this once per secret. Not once per PHP script execution.

[paragonie-security]

Another idea is to require this check before using the key material in any sign operations. See #164 for implementation.

[aidantwoods]

That seems like a decent place to catch this, given that the check is less expensive than a signature (but quite a bit more expensive than the length checks alone during import). I like the caching to avoid needing to do it every time too 🙂

I will note that: assuming an imported key is always used to sign something, the overall effort is the same whether we check on sign or on construction (we're just moving the compute really). This would help if people are importing keys and not using them though.

[paragonie-security]

If you always construct the object (because, say, you have a framework that loads a bunch of dependencies at bootstrap for some reason), the difference means a significant performance savings unless they actually need a signature.