random verification failure when using ECDSA with secp521r1

[adesitter]

Using:

// g++ -std=c++14 -Ijwt-cpp/include main_ec3.cpp -lcrypto

#include "jwt-cpp/jwt.h"

int main()
{
#if 1
// Fails randomly

//openssl ecparam -genkey -name secp521r1 | openssl ec -out myec.pem
//openssl ec -in myec.pem -pubout -out myec_pub.pem

const std::string ec_private_key = R"key(-----BEGIN EC PRIVATE KEY-----
MIHcAgEBBEIAVNNl9RB5yK3ZqXjoWlVDI/I3CngB6dVePYBHaEw05IzpsMyxp9NA
i+1vivgl25JB0DPACVU+LZ51W3MF1iPiuP6gBwYFK4EEACOhgYkDgYYABADnMhAz
X7tNRt4lxi7Npi68P4/5CYQe718O2XQvav+Bv+Os55JT/v5l3wLkmOAvQzWgUBdx
dd/P81kw91YcJT4w6AC9GOdGZQXEaqmqZwTSTY5nVQ54ejfdAS9CywSCCwYnLgbW
bQZQD/q/opNUNUzfX7oiYAZsd9CKpDr1hL6kip19zg==
-----END EC PRIVATE KEY-----)key";
const std::string ec_public_key = R"key(-----BEGIN PUBLIC KEY-----
MIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQA5zIQM1+7TUbeJcYuzaYuvD+P+QmE
Hu9fDtl0L2r/gb/jrOeSU/7+Zd8C5JjgL0M1oFAXcXXfz/NZMPdWHCU+MOgAvRjn
RmUFxGqpqmcE0k2OZ1UOeHo33QEvQssEggsGJy4G1m0GUA/6v6KTVDVM31+6ImAG
bHfQiqQ69YS+pIqdfc4=
-----END PUBLIC KEY-----)key";
#else
// Works reliably

//openssl ecparam -genkey -name secp256k1 | openssl ec -out myec.pem
//openssl ec -in myec.pem -pubout -out myec_pub.pem

const std::string ec_private_key = R"(-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgPGJGAm4X1fvBuC1z
SpO/4Izx6PXfNMaiKaS5RUkFqEGhRANCAARCBvmeksd3QGTrVs2eMrrfa7CYF+sX
sjyGg+Bo5mPKGH4Gs8M7oIvoP9pb/I85tdebtKlmiCZHAZE5w4DfJSV6
-----END PRIVATE KEY-----)";
const std::string ec_public_key = R"(-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEQgb5npLHd0Bk61bNnjK632uwmBfr
F7I8hoPgaOZjyhh+BrPDO6CL6D/aW/yPObXXm7SpZogmRwGROcOA3yUleg==
-----END PUBLIC KEY-----)";
#endif

if (false)
{
auto token = jwt::create()
.set_issuer("auth0")
.set_type("JWS")
.set_payload_claim("sample", jwt::claim(std::string("test")))
.sign(jwt::algorithm::es256{"", ec_private_key});
auto decoded = jwt::decode(token);

auto verifier = jwt::verify()
  .allow_algorithm(jwt::algorithm::es256{ ec_public_key })
  .with_issuer("auth0");

verifier.verify(decoded);

}
else
{
std::string hash = "012345678901234567890123456789012";

const jwt::algorithm::es256 signer("", ec_private_key);

const auto signature = signer.sign(hash);

const jwt::algorithm::es256 verifier(ec_public_key);

verifier.verify(hash, signature);

}
}
I observe random verification failures:

g++ -std=c++14 -Ijwt-cpp/include main_ec3.cpp -lcrypto
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
[ixdevenv] abgengcluster:/home/adesitter/MR2/EC_signing_verify>./a.out
terminate called after throwing an instance of 'jwt::signature_verification_exception'
what(): Invalid signature
Abort
g++ -std=c++14 -Ijwt-cpp/include main_ec3.cpp -lcrypto
./a.out
./a.out
./a.out
./a.out
./a.out
terminate called after throwing an instance of 'jwt::signature_verification_exception'
what(): Invalid signature
Abort

[Thalhammer]

Please take a look at the above bugfix.
It should fix your issue.
Turns out the padding of ecdsa signatures was wrong all along and no one noticed.
It only happened randomly as an ECDSA signature is not identical every time you generate it.
So the bug was not in verification but instead in generating.

[adesitter]

Thanks for the quick fix.
The case I provided works now as long as I use the correct algorithm (jwt::algorithm::es512).

A few comments:
#1 After the fix, the correct algorithm must to be called depending on the type of key passed by argument. Unless is can be addressed, There is now a case to be more specific in the arguments documentation.
In jwt.h:
* \param public_key ECDSA public key in PEM format
* \param private_key ECDSA private key or empty string if not available. If empty, signing will always fail.

"ECDSA with P-256 (or P-384 or, P-521)" (after https://tools.ietf.org/html/rfc7518#section-3.1)

Currently, using es256 with a P-521 key will result in an exception. It is possible to use es512 with a P-256 key ?

#2 In TokenTest.cpp, it may be worth testing jwt::algorithm::es512. Additionally, you could test that using es256 with a P-521 key result in an exception.

#3 Out of curiosity, it is there an explanation of the padding algorithm you are using somewhere ? It is somewhat puzzling that OpenSSL does not provide a function for this.

[Thalhammer]

@adesitter Regarding 1: It was never a feature that es256 worked with a 512bit key, just a coincident. In fact it probably was pure luck if the tokens were standard compliant.
No you can not (should not) use es512 with a 256 bit key.

Regarding 2: Yes testcases will come once I have time to write some. After they are done I will also add a new release.

Regarding 3: JWS uses a fixed signature length for ecdsa which consists of concatenating r and s of the signature. Therefore each of them needs to be half the size of that length. I basically prepend zeros until the size matches. Not sure why OpenSSL does not provide it, might do in the future but they also might ignore it as its really simple to implement and I could not find any other applications of this specific method.

[adesitter]

Thanks.
std::logic_error("bignum size exceeded expected length") could be replaced by std::invalid_argument("Incorrect key type").

[jfinkhaeuser]

I mean, you basically discussed this here already. I just added some details in #53 before crawling through the entire thread here :)