openssh unix dev - Dec 2014 - [PATCH] Early request for comments: U2F authentication

> 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 ((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".

When you change EVP_ecdsa() to EVP_sha256() above, EVP_VerifyFinal() 
should return "1" on valid data.


Best regards

    -Klaus

Thanks for pointing this out. Comments inline:

On Sun, Dec 14, 2014 at 11:42 PM, Klaus Keppler <kk at keppler-it.de>
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?

> If I do that, EVP_VerifyFinal() will result in EVP_R_WRONG_PUBLIC_KEY_TYPE.
This is strange... I don't get any error here, though I use the (same?) 
ECDSA public key from the attestation certificate (using OpenSSL 1.0.1i, 
but that shouldn't matter).
> 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.
Both EVP_PKEY_RSA_method and EVP_PKEY_ECDSA_method are #defined there as 
"EVP_PKEY_NULL_method". (don't ask me why... I don't
understand most of
that macro mess...)
> Unfortunately not. Could you share the code that you have please? Or is it
> not yet working?
Voila: https://github.com/keppler/fido-u2f/blob/master/fido-example.c
It uses the example messages from the official specs, so should be easy 
to reproduce.

If I'm wrong at any point there, please let me know.

Best regards

    -Klaus