gh-libtom/libtomcrypt
Watch
1
0
Fork
You've already forked libtomcrypt
 0
Code Issues 42 Pull requests 17 Releases 9 Wiki Activity

re-work PK crypto im- & export #302

Merged
sjaeckel merged 19 commits from fix/pr/301 into release/1.18.0 2017-10-02 07:05:59 -05:00
Conversation 18 Commits 19 Files changed 16 +86 -58
sjaeckel commented 2017-09-29 11:28:58 -05:00 (Migrated from github.com)

well this turned out to become a bigger one with some real bugfixes as well...

This mostly removes MAX_RSA_SIZE from the code, only rsa_make_key() still checks for it.

After some thinking I'd even propose to remove MIN_RSA_SIZEand MAX_RSA_SIZE entirely! Somebody against it?

well this turned out to become a bigger one with some real bugfixes as well... This mostly removes `MAX_RSA_SIZE` from the code, only `rsa_make_key()` still checks for it. After some thinking I'd even propose to remove `MIN_RSA_SIZE`and `MAX_RSA_SIZE` entirely! Somebody against it?
🎉 1
buggywhip (Migrated from github.com) reviewed 2017-09-29 11:28:58 -05:00
buggywhip commented 2017-09-29 13:26:39 -05:00 (Migrated from github.com)

On 29Sep, 2017, at 9:29 AM, Steffen Jaeckel notifications@github.com wrote:

After some thinking I'd even propose to remove MIN_RSA_SIZEand MAX_RSA_SIZE entirely! Somebody against it?

Not really, but note that it is currently set to 4096. Given recent interest in post-quantum resistance with longer key lengths, folks may want to play with 8192 and 15360. ...but given space and time, not for long. ;-)

That said, checking inputs for reasonableness is not a bad idea. The question then becomes what should be the limit? Everybody will have a different opinion but I consider 8192 another "reasonable" number. (A higher or lower number can always set it at compile time.)

Also on the subject of key lengths, a few lines further down in tomcrypt_custom.h, LTC_DER_MAX_PUBKEY_SIZE is set to 1024. FIPS.186-4 now defines longer DSA key lengths.
http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf
This might cause DSA key import problems in pk/dsa/dsa_import.c

> On 29Sep, 2017, at 9:29 AM, Steffen Jaeckel <notifications@github.com> wrote: > > After some thinking I'd even propose to remove MIN_RSA_SIZEand MAX_RSA_SIZE entirely! Somebody against it? Not really, but note that it is currently set to 4096. Given recent interest in post-quantum resistance with longer key lengths, folks may want to play with 8192 and 15360. ...but given space and time, not for long. ;-) That said, checking inputs for reasonableness is not a bad idea. The question then becomes what should be the limit? Everybody will have a different opinion but I consider 8192 another "reasonable" number. (A higher or lower number can always set it at compile time.) ----- Also on the subject of key lengths, a few lines further down in tomcrypt_custom.h, LTC_DER_MAX_PUBKEY_SIZE is set to 1024. FIPS.186-4 now defines longer DSA key lengths. http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf This might cause DSA key import problems in pk/dsa/dsa_import.c
karel-m commented 2017-09-29 14:08:07 -05:00 (Migrated from github.com)

I have doubts about der_decode_raw_bit_string:

1/ Ad blen/8 > *outlen - what if blen is not multiple of 8? And the
part *outlen = blen; perhaps also needs a fix.

2/ To me it seems like we are (falsely) assuming that out[] is
zero-initialized, we are setting just 1-bits.

I have doubts about `der_decode_raw_bit_string`: 1/ Ad `blen/8 > *outlen` - what if `blen` is not multiple of 8? And the part `*outlen = blen;` perhaps also needs a fix. 2/ To me it seems like we are (falsely) assuming that `out[]` is zero-initialized, we are setting just 1-bits.
sjaeckel commented 2017-09-29 18:32:28 -05:00 (Migrated from github.com)

Not really, but note that it is currently set to 4096. Given recent interest in post-quantum resistance with longer key lengths, folks may want to play with 8192 and 15360. ...but given space and time, not for long. ;-)

yeah, I would basically just remove the limits, so someone could create arbitrary sized RSA keys

That said, checking inputs for reasonableness is not a bad idea. The question then becomes what should be the limit? Everybody will have a different opinion but I consider 8192 another "reasonable" number. (A higher or lower number can always set it at compile time.)

I also think that sanity checking is useful at one point but I'm not sure if this library is the correct point where someone should hit an error because he chose an insane (from the library's view) value.

> Not really, but note that it is currently set to 4096. Given recent interest in post-quantum resistance with longer key lengths, folks may want to play with 8192 and 15360. ...but given space and time, not for long. ;-) yeah, I would basically just remove the limits, so someone could create arbitrary sized RSA keys > That said, checking inputs for reasonableness is not a bad idea. The question then becomes what should be the limit? Everybody will have a different opinion but I consider 8192 another "reasonable" number. (A higher or lower number can always set it at compile time.) I also think that sanity checking is useful at one point but I'm not sure if this library is the correct point where someone should hit an error because he chose an insane (from the library's view) value.
sjaeckel commented 2017-09-29 18:48:26 -05:00 (Migrated from github.com)

probably keeping MIN_RSA_SIZE but increasing it to 2048 is a good idea

probably keeping `MIN_RSA_SIZE` but increasing it to 2048 is a good idea
karel-m commented 2017-09-30 00:14:17 -05:00 (Migrated from github.com)

Ad (MAX|MIN)_RSA_SIZE I am for removing them completely.

Looking at der_decode_raw_bit_string once again:

outlen [in/out] The number of bits stored

So perhaps the original comparison with just blen was correct.

Ad (MAX|MIN)_RSA_SIZE I am for removing them completely. Looking at der_decode_raw_bit_string once again: outlen [in/out] The number of *bits* stored So perhaps the original comparison with just `blen` was correct.
rofl0r commented 2017-09-30 05:19:45 -05:00 (Migrated from github.com)

probably keeping MIN_RSA_SIZE but increasing it to 2048 is a good idea

wouldn't that prevent usage of 1024 bit keys ? i can imagine there are still some users of such keys, which may not have the option to change them.

> probably keeping MIN_RSA_SIZE but increasing it to 2048 is a good idea wouldn't that prevent usage of 1024 bit keys ? i can imagine there are still some users of such keys, which may not have the option to change them.
sjaeckel commented 2017-09-30 06:01:43 -05:00 (Migrated from github.com)

wouldn't that prevent usage of 1024 bit keys ? i can imagine there are still some users of such keys, which may not have the option to change them.

well it would prevent the creation of fresh 1024 bit keys but importing would still be feasible.

> wouldn't that prevent usage of 1024 bit keys ? i can imagine there are still some users of such keys, which may not have the option to change them. well it would prevent the creation of fresh 1024 bit keys but importing would still be feasible.
sjaeckel commented 2017-09-30 06:04:24 -05:00 (Migrated from github.com)

So perhaps the original comparison with just blen was correct.

yeah you're right... now the API is completely inconsistent..

> So perhaps the original comparison with just `blen` was correct. yeah you're right... now the API is completely inconsistent..
karel-m commented 2017-09-30 06:13:54 -05:00 (Migrated from github.com)

wouldn't that prevent usage of 1024 bit keys ? i can imagine there are
still some users of such keys, which may not have the option to change them.

well it would prevent the creation of fresh 1024 bit keys but importing
would still be feasible.

We do not have an ambition to prevent ltc users from weak crypo at many
other places. The recommended minimum should be mentioned with a loud
warning in the doc, not compiled into the library.

Enforcing min/max limits make sense to me only if those are limits of our
RSA make key implementation.

>>> wouldn't that prevent usage of 1024 bit keys ? i can imagine there are >>> still some users of such keys, which may not have the option to change them. >> >> well it would prevent the creation of fresh 1024 bit keys but importing >> would still be feasible. We do not have an ambition to prevent ltc users from weak crypo at many other places. The recommended minimum should be mentioned with a loud warning in the doc, not compiled into the library. Enforcing min/max limits make sense to me only if those are limits of our RSA make key implementation.
👍 1
rswindell commented 2017-09-30 21:03:55 -05:00 (Migrated from github.com)

For what it's worth, rsa_import() is now working in a (very) memory-constrained environment. Thanks for all your fixes!

For what it's worth, rsa_import() is now working in a (very) memory-constrained environment. Thanks for all your fixes!
karel-m commented 2017-10-01 07:21:41 -05:00 (Migrated from github.com)

According to my testing this PR introduces the following valgrind warnings:

==2355== 256 bytes in 1 blocks are indirectly lost in loss record 1 of 13
==2355==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==2355==    by 0x1C3688: mp_init (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18B069: init (ltm_desc.c:53)
==2355==    by 0x18C421: ltc_init_multi (multi.c:22)
==2355==    by 0x1A6370: rsa_make_key (rsa_make_key.c:70)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 768 bytes in 1 blocks are indirectly lost in loss record 3 of 13
==2355==    at 0x4C2DDCF: realloc (vg_replace_malloc.c:785)
==2355==    by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C74BF: s_mp_add (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C5CC3: mp_sub (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C8A3C: fast_mp_invmod (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C3838: mp_invmod (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18C001: invmod (ltm_desc.c:350)
==2355==    by 0x1A64CB: rsa_make_key (rsa_make_key.c:84)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 768 bytes in 1 blocks are indirectly lost in loss record 4 of 13
==2355==    at 0x4C2DDCF: realloc (vg_replace_malloc.c:785)
==2355==    by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C2453: mp_copy (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18B19E: copy (ltm_desc.c:77)
==2355==    by 0x1A64EF: rsa_make_key (rsa_make_key.c:86)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 768 bytes in 1 blocks are indirectly lost in loss record 5 of 13
==2355==    at 0x4C2DDCF: realloc (vg_replace_malloc.c:785)
==2355==    by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C2453: mp_copy (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18B19E: copy (ltm_desc.c:77)
==2355==    by 0x1A6513: rsa_make_key (rsa_make_key.c:87)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 7 of 13
==2355==    at 0x4C2DDCF: realloc (vg_replace_malloc.c:785)
==2355==    by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C74BF: s_mp_add (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C2046: mp_add (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C3BE4: mp_invmod_slow (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C3851: mp_invmod (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18C001: invmod (ltm_desc.c:350)
==2355==    by 0x1A63C9: rsa_make_key (rsa_make_key.c:75)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 8 of 13
==2355==    at 0x4C2DDCF: realloc (vg_replace_malloc.c:785)
==2355==    by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C8FF9: fast_s_mp_mul_digs (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C4DCE: mp_mul (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18B951: mul (ltm_desc.c:242)
==2355==    by 0x1A63F2: rsa_make_key (rsa_make_key.c:76)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 9 of 13
==2355==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==2355==    by 0x1C3737: mp_init_size (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C3637: mp_init_copy (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C27FA: mp_div (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18BAA4: divide (ltm_desc.c:265)
==2355==    by 0x1A6470: rsa_make_key (rsa_make_key.c:82)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 10 of 13
==2355==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==2355==    by 0x1C3737: mp_init_size (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C3637: mp_init_copy (in /home/noroot/libtomcrypt/test)
==2355==    by 0x1C27FA: mp_div (in /home/noroot/libtomcrypt/test)
==2355==    by 0x18BAA4: divide (ltm_desc.c:265)
==2355==    by 0x1A64A2: rsa_make_key (rsa_make_key.c:83)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

==2355== 6,848 (192 direct, 6,656 indirect) bytes in 8 blocks are definitely lost in loss record 13 of 13
==2355==    at 0x4C2DBC5: calloc (vg_replace_malloc.c:711)
==2355==    by 0x18B03D: init (ltm_desc.c:48)
==2355==    by 0x18C421: ltc_init_multi (multi.c:22)
==2355==    by 0x1A6370: rsa_make_key (rsa_make_key.c:70)
==2355==    by 0x126E7B: _rsa_issue_301 (rsa_test.c:319)
==2355==    by 0x12719A: rsa_test (rsa_test.c:368)
==2355==    by 0x1295F8: main (test.c:392)

The remaining 4 are DSA related and are unfortunately already present in release/1.18.0.

Steps to reproduce:

make -f makefile.unix clean
make -f makefile.unix CFLAGS="-g -DUSE_LTM -DLTM_DESC -I../libtommath" EXTRALIBS="../libtommath/libtommath.a" test
valgrind --error-exitcode=666 --leak-check=full --show-leak-kinds=all --errors-for-leak-kinds=all ./test
According to my testing this PR introduces the following valgrind warnings: ``` ==2355== 256 bytes in 1 blocks are indirectly lost in loss record 1 of 13 ==2355== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2355== by 0x1C3688: mp_init (in /home/noroot/libtomcrypt/test) ==2355== by 0x18B069: init (ltm_desc.c:53) ==2355== by 0x18C421: ltc_init_multi (multi.c:22) ==2355== by 0x1A6370: rsa_make_key (rsa_make_key.c:70) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 768 bytes in 1 blocks are indirectly lost in loss record 3 of 13 ==2355== at 0x4C2DDCF: realloc (vg_replace_malloc.c:785) ==2355== by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C74BF: s_mp_add (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C5CC3: mp_sub (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C8A3C: fast_mp_invmod (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C3838: mp_invmod (in /home/noroot/libtomcrypt/test) ==2355== by 0x18C001: invmod (ltm_desc.c:350) ==2355== by 0x1A64CB: rsa_make_key (rsa_make_key.c:84) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 768 bytes in 1 blocks are indirectly lost in loss record 4 of 13 ==2355== at 0x4C2DDCF: realloc (vg_replace_malloc.c:785) ==2355== by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C2453: mp_copy (in /home/noroot/libtomcrypt/test) ==2355== by 0x18B19E: copy (ltm_desc.c:77) ==2355== by 0x1A64EF: rsa_make_key (rsa_make_key.c:86) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 768 bytes in 1 blocks are indirectly lost in loss record 5 of 13 ==2355== at 0x4C2DDCF: realloc (vg_replace_malloc.c:785) ==2355== by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C2453: mp_copy (in /home/noroot/libtomcrypt/test) ==2355== by 0x18B19E: copy (ltm_desc.c:77) ==2355== by 0x1A6513: rsa_make_key (rsa_make_key.c:87) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 7 of 13 ==2355== at 0x4C2DDCF: realloc (vg_replace_malloc.c:785) ==2355== by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C74BF: s_mp_add (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C2046: mp_add (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C3BE4: mp_invmod_slow (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C3851: mp_invmod (in /home/noroot/libtomcrypt/test) ==2355== by 0x18C001: invmod (ltm_desc.c:350) ==2355== by 0x1A63C9: rsa_make_key (rsa_make_key.c:75) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 8 of 13 ==2355== at 0x4C2DDCF: realloc (vg_replace_malloc.c:785) ==2355== by 0x1C35AC: mp_grow (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C8FF9: fast_s_mp_mul_digs (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C4DCE: mp_mul (in /home/noroot/libtomcrypt/test) ==2355== by 0x18B951: mul (ltm_desc.c:242) ==2355== by 0x1A63F2: rsa_make_key (rsa_make_key.c:76) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 9 of 13 ==2355== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2355== by 0x1C3737: mp_init_size (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C3637: mp_init_copy (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C27FA: mp_div (in /home/noroot/libtomcrypt/test) ==2355== by 0x18BAA4: divide (ltm_desc.c:265) ==2355== by 0x1A6470: rsa_make_key (rsa_make_key.c:82) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 1,024 bytes in 1 blocks are indirectly lost in loss record 10 of 13 ==2355== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2355== by 0x1C3737: mp_init_size (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C3637: mp_init_copy (in /home/noroot/libtomcrypt/test) ==2355== by 0x1C27FA: mp_div (in /home/noroot/libtomcrypt/test) ==2355== by 0x18BAA4: divide (ltm_desc.c:265) ==2355== by 0x1A64A2: rsa_make_key (rsa_make_key.c:83) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ==2355== 6,848 (192 direct, 6,656 indirect) bytes in 8 blocks are definitely lost in loss record 13 of 13 ==2355== at 0x4C2DBC5: calloc (vg_replace_malloc.c:711) ==2355== by 0x18B03D: init (ltm_desc.c:48) ==2355== by 0x18C421: ltc_init_multi (multi.c:22) ==2355== by 0x1A6370: rsa_make_key (rsa_make_key.c:70) ==2355== by 0x126E7B: _rsa_issue_301 (rsa_test.c:319) ==2355== by 0x12719A: rsa_test (rsa_test.c:368) ==2355== by 0x1295F8: main (test.c:392) ``` The remaining 4 are DSA related and are unfortunately already present in `release/1.18.0`. Steps to reproduce: ``` make -f makefile.unix clean make -f makefile.unix CFLAGS="-g -DUSE_LTM -DLTM_DESC -I../libtommath" EXTRALIBS="../libtommath/libtommath.a" test valgrind --error-exitcode=666 --leak-check=full --show-leak-kinds=all --errors-for-leak-kinds=all ./test ```
karel-m (Migrated from github.com) reviewed 2017-10-01 07:43:29 -05:00
tests/rsa_test.c
@ -288,0 +313,4 @@
static int _rsa_issue_301(int prng_idx)
{
rsa_key key, key_in;
unsigned char buf[4096];
karel-m (Migrated from github.com) commented 2017-10-01 07:43:29 -05:00

approx buf[2500] should be enough here

approx `buf[2500]` should be enough here
karel-m (Migrated from github.com) reviewed 2017-10-01 07:44:00 -05:00
tests/rsa_test.c
@ -288,0 +316,4 @@
unsigned char buf[4096];
unsigned long len;
DO(rsa_make_key(&yarrow_prng, prng_idx, sizeof(buf)/8, 65537, &key));
karel-m (Migrated from github.com) commented 2017-10-01 07:44:00 -05:00

4096/8 instead of sizeof(buf)/8 would be IMO better

`4096/8` instead of `sizeof(buf)/8` would be IMO better
karel-m (Migrated from github.com) reviewed 2017-10-01 07:45:39 -05:00
tests/rsa_test.c
karel-m (Migrated from github.com) commented 2017-10-01 07:45:39 -05:00

what about DO(_rsa_issue_301(prng_idx))?

what about `DO(_rsa_issue_301(prng_idx))`?
karel-m (Migrated from github.com) reviewed 2017-10-01 07:47:21 -05:00
src/pk/rsa/rsa_make_key.c
@ -31,10 +31,7 @@ int rsa_make_key(prng_state *prng, int wprng, int size, long e, rsa_key *key)
LTC_ARGCHK(ltc_mp.name != NULL);
karel-m (Migrated from github.com) commented 2017-10-01 07:47:21 -05:00

considering the fact that size is int we should at least check LTC_ARGCHK(size > 0)

considering the fact that `size` is `int` we should at least check `LTC_ARGCHK(size > 0)`
karel-m (Migrated from github.com) reviewed 2017-10-01 07:48:27 -05:00
tests/rsa_test.c
@ -285,6 +285,64 @@ static int rsa_compat_test(void)
return 0;
karel-m (Migrated from github.com) commented 2017-10-01 07:48:27 -05:00

what about return CRYPT_OK instead of return 0

what about `return CRYPT_OK` instead of `return 0`
karel-m (Migrated from github.com) approved these changes 2017-10-02 04:05:52 -05:00
karel-m (Migrated from github.com) left a comment

👍

👍
Sign in to join this conversation.
Reviewers
No reviewers
buggywhip
karel-m
Labels
Clear labels   
bug

   
duplicate

   
enhancement

   
feature

   
help wanted

   
improve API

   
invalid

   
needs documentation

   
not-planned
This ticket is not planned to be implemented and/or fixed from the core team.

  
rtfm

   
stale

   
support

   
wishlist
No labels
bug
duplicate
enhancement
feature
help wanted
improve API
invalid
needs documentation
not-planned
rtfm
stale
support
wishlist
Milestone
Clear milestone
No items
No milestone
v1.18.0
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libtom/libtomcrypt!302
No description provided.