Thanks for pointing this out. Comments inline:
On Sun, Dec 14, 2014 at 11:42 PM, Klaus Keppler <kk@xxxxxxxxxxxxx> wrote:
> I’ve spent some time (together with Christian and Thomas) hacking on
>> U2F support in OpenSSH, and I’m happy to provide a first patch — it’s
>> not complete, but it should be good enough to get the discussion going
>> :). Please see the two attached files for the patch.
>>
>
> This is great - I'm looking forward to it! :)
>
> I've implemented U2F into another (C-based) application these days. While
> searching for some relevant OpenSSL-specific "help" I stumbled upon your
> OpenSSH patch.
> I think there's a small bug:
>
> + if ((err = EVP_VerifyInit(&mdctx, EVP_ecdsa())) != 1) {
>> + ERR_error_string(ERR_get_error(), errorbuf);
>> + fatal("EVP_VerifyInit() failed: %s (reason: %s)",
>> + errorbuf, ERR_reason_error_string(err));
>>
>
> You should use "EVP_sha256()" instead of "EVP_ecdsa()" here (we have a
> ECDSA signature on the SHA256 hash)
>
If I do that, EVP_VerifyFinal() will result in EVP_R_WRONG_PUBLIC_KEY_TYPE.
Looking at the OpenSSL source, I can see that in crypto/evp/m_sha1.c, the
sha* digests are defined with EVP_PKEY_RSA_method, which requires an RSA
publickey, but we have an ECDSA publickey. The only digest working with
ECDSA publickeys is crypto/evp/m_ecdsa.c AFAICT.
>
> + if ((err = EVP_VerifyFinal(&mdctx, walk, restlen, pkey)) == -1) {
>> + ERR_error_string(ERR_get_error(), errorbuf);
>> + error("Verifying the U2F registration signature failed:
>> %s (reason: %s)",
>> + errorbuf, ERR_reason_error_string(err));
>> + goto out;
>> + }
>>
>
> You test EVP_VerifyFinal() only against "-1". This catches OpenSSL library
> errors and such. But if the signature check itself fails, you get "0". So,
> the only valid result here should be "1".
>
You’re correct, good catch.
>
> When you change EVP_ecdsa() to EVP_sha256() above, EVP_VerifyFinal()
> should return "1" on valid data.
>
Unfortunately not. Could you share the code that you have please? Or is it
not yet working?
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
