943639 - Support for Brainpool ECC Curve (rfc5639)

Status: RESOLVED WONTFIX

(NSS :: Libraries, defect, P5)

Description

[Kurt Roeckx]

I think it would be nice if there was support for the brainpool curves as alternative to the nist p curves currently supported.

Comment 1

[Kurt Roeckx]

There of course is also rfc7027 for using that in TLS.

Comment 2

[MiW CryptoCurrency]

Hi,

I have implemented these curves in NSS. My working patch is here.
I have tested this patch against all 28 curves in both ECDHE and ECDSA signed certificates.

https://github.com/MiWCryptoCurrency/CertificatesInBrowsers/blob/master/TLS25%2BBrainpool-Firefox-NSS.patch

This patch contains the other 22 curve parameters and the fix for the ssl3_SuiteBOnly function so would not be suitable yet for inclusion. Will submit clean brainpool only patch soon. It also requires environment definition NSS_ECC_MORE_THAN_SUITE_B to ensure the additional code is compiled in.
This patch implements rfc7027 and the other curves in rfc4492

The only other concern I have with this patch is that it requires bypassing the default case of ec_GFp_sub_6 (384 bit optimized) for field subtraction, as I found this function returns incorrect values sometimes on the brainpoolP384r1 field.
The other secg384r1 curve does not seem to show the same behavior.

This causes troubles as both affine and mont mul arithmetic, which returns different and incorrect values.
It also has a new GF definition I used for testing affine mul; I have #ifdef'd it with the ECL_ENABLE_GFP_PT_MUL_AFF to match the actual affine multiplication that is not compiled by default.

/* use generic subtraction methods */
group->meth->field_sub = &ec_GFp_sub;

Thanks for the support from awesome people.

Comment 3

[MiW CryptoCurrency]

Attachment: Add support for EC brainpoolP256r1, brainpoolP384r1, brainpoolP512r1 for ECDSA and ECDHE

Adds support for brainpool while operating in Extended ECC mode (NSS_ECC_MORE_THAN_SUITE_B).

issues:
* Will require the missing 22 curve parameters from ecl-curve.h
* BP384r1 overrides default case of using group meth->field_sub = ec_GFp_sub_6 to use generic ec_GFp_sub because of arithmetic errors

Tested with OpenSSL 1.0.2 over various certificates with brainpool EC public keys (ECDSA) and named_curve (ECDHE).

Comment 4

[MiW CryptoCurrency]

I have discovered the cause of the need to override the default case of meth->field_sub = ec_GFp_sub_6 in brainpoolP384r1.

ec_GFp_sub_6 will return an incorrect subtraction result if the most significant 64 bits of prime are not 0xFFFFFFFFFFFFFFFF in the event of sub_borrow carry. This is due to a missing MP_ADD_CARRY(b5, r5, r5, borrow, borrow).

group NISTP384 uses this function, but as the prime is of that form the bug does not manifest.
I have documented this in https://bugzilla.mozilla.org/show_bug.cgi?id=1128140
NIST P256 and P521 are not of this above form, but use different field subtraction functions.
Brainpool P384 uses 0x8CB91E82A3386D28 as a MS64b

proposed ssl3_SuiteBOnly bug fix to be submitted soon.