Generated EC key has wrong length

[rlipscombe]

Describe the bug
EC private keys on the secp256r1 curve should have 32-byte private keys. public_key:generate_key/1 occasionally generates 31-byte keys.

To Reproduce

Bad = lists:foldl(fun(_, Acc) ->
        Key = public_key:generate_key({namedCurve, secp256r1}),
        PrivateKey = case Key of
            % record changed between OTP-23 and OTP-24.
            {'ECPrivateKey', 1, K, _Parameters, _PublicKey, _} -> K;
            {'ECPrivateKey', 1, K, _Parameters, _PublicKey} -> K
        end,
        case byte_size(PrivateKey) of
            32 -> Acc;
            _ -> [Key | Acc]
        end
    end, [], lists:seq(1, 10_000)).
0 = length(Bad).

Expected behavior
The private key (binary) should be zero-prefixed to the correct length.

References

RFC5915 (section 3) -- https://www.rfc-editor.org/rfc/rfc5915.html#section-3

privateKey is the private key.  It is an octet string of length ceiling (log2(n)/8) (where n is the order of the curve) obtained from the unsigned integer via the Integer-to-Octet-String-Primitive (I2OSP) defined in [RFC3447]

RFC3447 (section 4.1) -- https://www.rfc-editor.org/rfc/rfc3447#section-4.1

note that one or more leading digits will be zero if [padding is needed]

Affected versions
OTP-23.0.3
OTP-24.0

[rlipscombe]

In 10,000 iterations (above), it generates 30-50 "bad" keys.

[HansN]

OTP-22.0 is also affected and probably all previous versions.

[rlipscombe]

I did some digging. The public_key:generate_key/1 call fairly quickly ends up in the crypto NIF, and eventually at this line here:

    priv_key = bn2term(env, EC_KEY_get0_private_key(key));

EC_KEY_get0_private_key returns a BIGNUM; bn2term is responsible for converting it to a binary term, but bn2term doesn't (and cannot) know that it should be zero-prefixing the result.

[rlipscombe]

More digging. afaict, the private key should be the same length as the "order" of the curve, which, for the named curves, is defined in crypto_ec_curves:curve/1. This is passed to crypto:ec_key_generate/2, which is a NIF, defined in ec.c. It's this NIF that calls bn2term mentioned previously.

[HansN]

Fixed in maint and master, and a patch is planned to be released in:

• OTP 24.0.2,
• OTP 23.3.4.2 and
• OTP 22.3.4.20

Thanks for the report @rlipscombe !