Add RSA key generation [ERL-165]

[wiml]

This adds support for RSA key generation using generate_key(rsa, {bits, e}) (see the ERL-165 bug comments section).

Key generation is slow, so this relies on being able to schedule the computation using ERL_NIF_DIRTY_JOB_CPU_BOUND. The test case should be automatically skipped if dirty schedulers aren't available.

I've tested this on an erlang linked against openssl 1.0.1, but it should work with 0.9.8 and 1.1.0 as well.

[HansN]

The generate/1 test case fails on FIPS 140-2 enabled libs for the 1024 length keys. See the case ?config(fips, Config) of ... end a few lines above for inspiration of a fix. I haven't found any clear indication anywhere whether the 2k length is enough, but the 1k is surely to short in the FIPS case.

[wiml]

Ahh. Chasing down some references, it looks like the required minimum key length for FIPS 140-2 validation is currently 2048 bits (from "FCS_CKM.1.1(1)" here).

There's probably no need to test key generation with a huge number of parameters; I'll just remove the 1024-bit test case.

[sverker]

Two points on the documentation:

1. Dirty schedulers will lose there "experimental" status in OTP-20, so you can remove that word.

2. We will change enif_schedule_nif to raise 'notsup' exception instead of 'badarg' if dirty schedulers are not enabled. And I think this is more correct for crypto:generate_key. So, why not mention the 'notsup' exception in the note about dirty scheduler support.

[sverker]

Our valgrind tests complain about ERR_load_crypto_strings() leaking memory.

Seems to me like a call to ERR_free_string() in unload() when --library_refc == 0 should fix that.

[HansN]

Unfortunatly it still fails on the FIPS-enabled machine. Maybe 2k should be removed also?

[wiml]

Hm. 2k should be okay, from everything I've read, so I'd rather not remove the test without a better understanding of why it's failing. What error message does it throw?

I'll see if I can build a FIPS build and investigate locally, but I probably won't have time for a few days / a week.

[HansN]

Strange... Unfortunately the test logs has been removed because they are kept only a couple of days. I've added the branch to testing again so we can see tomorrow what the message was.

[HansN]

With fips mode 2k gives error, but 3k does not. The complaint is on the e value, not the modulus wich I didn't expect. Does FIPS prescribe a different algorithm for generate_key so the e becomes invalid?

14> crypto:enable_fips_mode(false).                                                                                                              
true
15> f(),{Pub,Priv} = crypto:generate_key(rsa, {2048,3}), S = crypto:sign(rsa, sha256, <<"Hello world">>, Priv), crypto:verify(rsa, sha256, <<"Hello world">>, S, Pub).     
true
16> f(),{Pub,Priv} = crypto:generate_key(rsa, {3072,65537}), S = crypto:sign(rsa, sha256, <<"Hello world">>, Priv), crypto:verify(rsa, sha256, <<"Hello world">>, S, Pub).
true
17> 
17> 
17> crypto:enable_fips_mode(true).                                                                                                               true                     
18> f(),{Pub,Priv} = crypto:generate_key(rsa, {2048,3}), S = crypto:sign(rsa, sha256, <<"Hej hopp">>, Priv), {S, crypto:verify(rsa, sha256, <<"Hej hopp">>, S, Pub)}.     
** exception error: {error,{openssl,[{"rsa routines","RSA_BUILTIN_KEYGEN",
                                      "bad e value"}]}}
     in function  crypto:rsa_generate_key_nif/2
        called as crypto:rsa_generate_key_nif(2048,<<3>>)
     in call from crypto:generate_key/3 (crypto.erl, line 456)
19> f(),{Pub,Priv} = crypto:generate_key(rsa, {3072,65537}), S = crypto:sign(rsa, sha256, <<"Hello world">>, Priv), crypto:verify(rsa, sha256, <<"Hello world">>, S, Pub).
true
20> crypto:info_lib().
[{<<"OpenSSL">>,268439711,
  <<"OpenSSL 1.0.1i-fips 6 Aug 2014">>}]
21> 

[wiml]

Ah! I understand now. Using a small public exponent e used to be considered a problem; I guess FIPS prohibits it. I'll change the test case to {2048,17}.

[HansN]

Great! You found it, but {2048,17} still fails. However {2048,65537} works!
And don't forget the Valgrind fix that Sverker commented on.

Just for your information: when I test this in the shell, with fips disabled it takes 0.2-0.3 s to generate the key pair, but with fips enabled 0.9-1.3 s! So the fips mode does more than simply checking the sizes.

[HansN]

For fun I just checked {1024,65537} and that works in fips mode.! But {512,65537} gives expected error:

 {error,{openssl,[{"FIPS routines","RSA_BUILTIN_KEYGEN",
                                      "key too short"}]}}

[sverker]

About ERR_load_crypto_strings() leaking memory. I don't think calling ERR_free_string() in unload will be correct as there can be two (current and old) different crypto.so loaded at the same time sharing the same libcrypto.so. We can't call ERR_free_string() until libcrypto.so is unloaded and that is not easy to know when it happens.

Any ideas?

[wiml]

@sverker - It looks like the library_refc maintained by load/upgrade/unload is enough to deal with that. Experimentally, the refcount only goes to zero when the emulator is shutting down. I've added calls to ERR_free_strings() and CRYPTO_cleanup_all_ex_data() when refc goes to zero and this makes valgrind happier.

I'm a little concerned about the possibility of the same libcrypto.so being loaded (indirectly) by some other erlang module, and being broken when crypto.so de-initalizes libcrypto.so. I'm not sure how to handle this, other than that openssl 1.1.0 claims to do the necessary de-initialization internally so that the problem doesn't occur.

[sverker]

No, library_refc will not solve this for all cases.
Consider your concern about "other" modules with dependencies to libcrypto.so. What I tried to explain is that the "other" module can actually be a different version of crypto.so. The module upgrade mechanics in Erlang allows two versions ("current" and "old") of the same module be loaded and executing at the same time. If two crypto versions use the same crypto.so then they will share the same static library_refc variable and it will get the value 2; no problem. But if the two crypto versions use different versions of crypto.so then they will get individual instances of static variable library_refc, each with value 1. When the "old" crypto version is purged, unload() is called and it will count library_refc down to 0, call ERR_free_strings() while the "current" crypto module is still using libcrypto.so; problem.

If OpenSSL 1.1.0 solves this by doing automatic de-initialization, then one solution could be to skip the fancy error messages for older versions.

[HansN]

Any activity on this PR?