Update LibTomMath to 1.2.0 #84

sjaeckel · 2019-10-25T15:55:32Z

Title says it all

mkj · 2019-11-05T14:11:32Z

Thanks. I'll investigate the memory leak found at https://travis-ci.org/mkj/dropbear/jobs/602889758#L21324 before merging.

mkj · 2020-05-25T16:16:38Z

Has libtommath changed how mp_init allocates memory? It's now leaking the allocation from
https://github.com/libtom/dropbear/blob/ltm-1.2.0/svr-kex.c#L209 send_msg_kexdh_reply
when it exits via
https://github.com/libtom/dropbear/blob/ltm-1.2.0/svr-kex.c#L210 send_msg_kexdh_reply
https://github.com/libtom/dropbear/blob/ltm-1.2.0/common-kex.c#L669 kexecdh_comb_key
https://github.com/libtom/dropbear/blob/ltm-1.2.0/ecc.c#L162 buf_get_ecc_raw_pubkey

That's fine for it to exit, but I don't see why it didn't leak previously. I'll dig into it later, but just in case you have any immediate thoughts.

 LSAN_OPTIONS=verbosity=1:log_threads=1 ./fuzzer-preauth  -v ../dropbear-fuzzcorpus/fuzzer-preauth/0709ed56265156cdc42b69a68bb99c5651adc061
...
TRACE  (17374) 0.441299: process_packet: packet type = 30,  len 42
TRACE  (17374) 0.441315: got expected packet 30 during kexinit
TRACE  (17374) 0.441321: enter recv_msg_kexdh_init
TRACE  (17374) 0.441329: enter send_msg_kexdh_reply
TRACE  (17374) 0.471533: enter buf_get_ecc_raw_pubkey
TRACE  (17374) 0.471543: leave, wrong size
Exit before auth from <fuzzremotehost:9876>: (user 'matt', 1 fails): ECC error
TRACE  (17374) 0.471560: enter session_cleanup
TRACE  (17374) 0.471563: enter chancleanup
TRACE  (17374) 0.471569: leave chancleanup
TRACE  (17374) 0.471628: leave session_cleanup
TRACE  (17374) 0.471644: dropbear_exit longjmped
Done ../dropbear-fuzzcorpus/fuzzer-preauth/0709ed56265156cdc42b69a68bb99c5651adc061
Finished
==17376==Processing thread 17374.
==17376==Stack at 0x7ffdd1977000-0x7ffdd2177000 (SP = 0x7ffdd2174c78).
==17376==TLS at 0x7f58b50a7d80-0x7f58b50a8e00.

=================================================================
==17374==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 768 byte(s) in 3 object(s) allocated from:
    #0 0x7f58b3fc5d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x55da317168df in mp_init /home/matt/src/dropbear-git/libtommath/bn_mp_init.c:10
    #2 0x55da31703120 in ltc_init_multi src/math/multi.c:22
    #3 0x55da3170851d in ecc_make_key_ex src/pk/ecc/ecc_make_key.c:83
    #4 0x55da316abf17 in gen_kexecdh_param /home/matt/src/dropbear-git/common-kex.c:652
    #5 0x55da316ea91d in send_msg_kexdh_reply /home/matt/src/dropbear-git/svr-kex.c:214
    #6 0x55da316e9f76 in recv_msg_kexdh_init /home/matt/src/dropbear-git/svr-kex.c:80
    #7 0x55da316d7fe1 in process_packet /home/matt/src/dropbear-git/process-packet.c:146
    #8 0x55da316af29a in session_loop /home/matt/src/dropbear-git/common-session.c:254
    #9 0x55da316ed664 in svr_session /home/matt/src/dropbear-git/svr-session.c:200
    #10 0x55da316c773a in fuzz_run_preauth /home/matt/src/dropbear-git/fuzz-common.c:190
    #11 0x55da3168e5a1 in LLVMFuzzerTestOneInput /home/matt/src/dropbear-git/fuzzer-preauth.c:4
    #12 0x55da3172d74f in main /home/matt/src/dropbear-git/fuzz-harness.c:35
    #13 0x7f58b32a7b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Direct leak of 768 byte(s) in 3 object(s) allocated from:
    #0 0x7f58b3fc5d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x55da317168df in mp_init /home/matt/src/dropbear-git/libtommath/bn_mp_init.c:10
    #2 0x55da31703120 in ltc_init_multi src/math/multi.c:22
    #3 0x55da3170851d in ecc_make_key_ex src/pk/ecc/ecc_make_key.c:83
    #4 0x55da316abf17 in gen_kexecdh_param /home/matt/src/dropbear-git/common-kex.c:652
    #5 0x55da316ea91d in send_msg_kexdh_reply /home/matt/src/dropbear-git/svr-kex.c:214
    #6 0x55da316e9f76 in recv_msg_kexdh_init /home/matt/src/dropbear-git/svr-kex.c:80
    #7 0x55da316d7fe1 in process_packet /home/matt/src/dropbear-git/process-packet.c:146
    #8 0x55da316af29a in session_loop /home/matt/src/dropbear-git/common-session.c:254
    #9 0x55da316ed664 in svr_session /home/matt/src/dropbear-git/svr-session.c:200
    #10 0x55da316c773a in fuzz_run_preauth /home/matt/src/dropbear-git/fuzz-common.c:190
    #11 0x55da3168e5a1 in LLVMFuzzerTestOneInput /home/matt/src/dropbear-git/fuzzer-preauth.c:4
    #12 0x55da3172d7d6 in main /home/matt/src/dropbear-git/fuzz-harness.c:37
    #13 0x7f58b32a7b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Direct leak of 160 byte(s) in 2 object(s) allocated from:
    #0 0x7f58b3fc5f30 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdef30)
    #1 0x55da31716799 in mp_grow /home/matt/src/dropbear-git/libtommath/bn_mp_grow.c:20

SUMMARY: AddressSanitizer: 1696 byte(s) leaked in 8 allocation(s).

sjaeckel · 2020-05-25T16:49:50Z

after you said mp_init I thought ah right, there was something changed...

themiron · 2020-06-18T10:27:00Z

@sjaeckel @mkj this update increases dropber by ~10Kb with same -Os optimization options (~5% on openwrt of resulting bin)
how do you think, can be something done about?

mkj · 2020-06-18T11:14:36Z

@themiron I missed that Toom and Karatsuba names had changed, 10e119f saves ~4KB on Linux x64. I'll have to see what else increased the size.
What platform is your ~10kB on?

themiron · 2020-06-18T12:36:22Z

@mkj thanks.
btw, maybe just undefine BN_DEPRECATED_C and drop deprecated BN_MP_KARATSUBA*/TOOM* undefines, just for consistency?
also, MP_FIXED_CUTOFFS can be defined, saves 16 bytes, cutoffs are not changed in runtime anyway.
platform - openwrt, mips32 24kc gcc-8.4.4 musl, checking the rest growth, will feedback later

sjaeckel · 2020-06-18T15:38:54Z

@minad maybe you can give some more advice on size optimization of ltm?

minad · 2020-06-18T15:56:08Z

I am unsure as to the reasons of the size changes. Does dropbear use a custom configuration? Generally the optimal size can be achieved by using an amalgamated build with everything unused disabled. This is at least what I did, but it requires quite a bit of effort.

minad · 2020-06-18T15:57:45Z

...also all functions should be marked static in the amalgamated build in order for the optimizer to do its best. But lto or thin-lto with function sections will get you almost there too.

themiron · 2020-06-19T00:52:51Z

well, sorry, I did compare ltm 1.2.0 with 2019.78 release state, not with previous state (1.1.0).
current git on 1.1.0 (static inlines for 1.2.0 api compat) vs current git with 1.2.0 gives only 356 bytes.
seems size increase (excluding Toom and Karatsuba) has happened earlier.

btw, libtommath uses own rand routines inside mp_rand, maybe hook it into dropbear's genrandom() and disable s_mp_rand_platform() as well?

output from https://git.busybox.net/busybox/tree/scripts/bloat-o-meter script:

s_mp_exptmod_fast                                      -    2280   +2280
mp_mul                                               188    1036    +848
s_mp_montgomery_reduce_fast                            -     744    +744
s_mp_sqr_fast                                          -     540    +540
s_mp_mul_digs_fast                                     -     464    +464
s_mp_mul_high_digs_fast                                -     448    +448
mp_to_ubin                                             -     276    +276
s_mp_rand_platform                                     -     272    +272
mp_from_ubin                                           -     240    +240
static.printhelp                                      40     148    +108
mp_set_i32                                             -     100    +100
mp_set_u32                                             -      88     +88
mp_ubin_size                                           -      64     +64
.rodata                                            24792   24856     +64
sizes                                                112     160     +48
static.mp_div_d                                      764     784     +20
mp_clear                                             104     116     +12
static.mp_read_radix                                 356     364      +8
mp_lshd                                              236     244      +8
mp_mul_d                                             312     316      +4
mp_mod_2d                                            296     300      +4
getrsaprime                                          540     544      +4
mp_rshd                                              152     148      -4
mp_invmod                                           2500    2496      -4
mp_2expt                                             160     156      -4
mp_prime_miller_rabin                                736     728      -8
mp_zero                                               60      48     -12
mp_set                                                84      72     -12
mp_init_size                                         156     140     -16
mp_sub_d                                             452     432     -20
mp_init                                              116      96     -20
mp_grow                                              184     164     -20
buf_put_sign                                        1088    1068     -20
mp_add_d                                             496     472     -24
mp_div                                              1828    1800     -28
buf_putmpint                                         332     292     -40
mp_exptmod                                           712     660     -52
mp_rand                                              300     236     -64
mp_unsigned_bin_size                                  72       -     -72
mp_set_long                                          204       -    -204
s_read_dev_urandom                                   244       -    -244
mp_read_unsigned_bin                                 244       -    -244
mp_to_unsigned_bin                                   260       -    -260
static.mp_prime_next_prime                          6276    5836    -440
fast_s_mp_mul_high_digs                              448       -    -448
fast_s_mp_mul_digs                                   464       -    -464
fast_s_mp_sqr                                        540       -    -540
fast_mp_montgomery_reduce                            744       -    -744
mp_exptmod_fast                                     2280       -   -2280
------------------------------------------------------------------------------
(add/remove: 11/10 grow/shrink: 11/17 up/down: 6644/-6288)    Total: 356 bytes```

mkj · 2020-06-25T16:08:28Z

@minad thanks, LTO is likely worthwhile. Also trying -ffunction-sections -fdata-sections -Wl,--gc-sections.

Setting LTM_USE_ONLY_MR saves ~2.7kB on x64 (probably more in MIPS iirc?), but not sure if it's only a time tradeoff or something else too. @themiron thanks for the pointer on random, saves around 200 bytes there with some #ifdefs.

sjaeckel · 2020-06-25T17:11:39Z

Enabling LTM_USE_ONLY_MR disables the lucas-selfridge primality test and I'm not sure anymore whether the numbers returned by mp_prime_rabin_miller_trials() are fine to make the prime check FIPS 186.4 compliant ...

@czurnieden we discussed whether it would make sense to provide a second table in mp_prime_rabin_miller_trials.c in case LTM_USE_ONLY_MR is defined, you said no in libtom/libtommath#364, now that I look at the FIPS pdf again I'd still say "we need another table in that case" ... but maybe I'm also too paranoid ... :) can you please have a look (again)

czurnieden · 2020-06-26T06:26:19Z

@sjaeckel

we discussed whether it would make sense to provide a second table in mp_prime_rabin_miller_trials.c in case LTM_USE_ONLY_MR is defined, you said no in libtom/libtommath#364, now that I look at the FIPS pdf again I'd still say "we need another table in that case"

A short summary for those here who are not familiar with the algorithm:

The default test (strong BPSW) uses M-R rounds with bases 2 and 3 at the beginning and one round with a random base at the end if t==0. The large number of rounds in mp_prime_rabin_miller_trials are not needed; t == 3 would already be a very conservative value.

If LTM_USE_ONLY_MR is defined, the bases 2 and 3 are always used (these bases are convenient to sieve out a large amount). A positive number given to t in mp_prime_is_prime(const mp_int *a, int t, bool *result) will run t M-R tests with random bases with at least one test even if t==0. The necessary number of trials should be taken from mp_prime_rabin_miller_trials.

I'm not sure anymore whether the numbers returned by mp_prime_rabin_miller_trials() are fine to make the prime check FIPS 186.4 compliant ...

The numbers themself are FIPS 186.4 compliant. The correctness of those numbers can be proven if necessary.

I think it should be noted that the table is for RSA only, DSA needs much lower errors. I would recommend to use BPSW and additionally set t from the table in that case or with ceil( -log_2(p)/2 ) (where p is the probability that the number is composite (e.g.: 2^(-100))) for LTM_USE_ONLY_MR.

... but maybe I'm also too paranoid ... :)

One cannot be too paranoid with the implementation of cryptography. Borderline clinical is still OK.

sjaeckel mentioned this pull request Oct 25, 2019

Update to list of number of Miller-Rabin trials #82

Closed

sjaeckel force-pushed the ltm-1.2.0 branch from 8d65188 to e15b565 Compare May 25, 2020 16:30

sjaeckel added 8 commits May 25, 2020 18:32

update C files

33f5652

update other files

8f8b7ca

update headers

0dc57d5

update makefiles

9b38703

remove mp_set/get_double()

b4686df

use ltm 1.2.0 API

9d9f45e

update ltm_desc

a5887bb

use bundled tommath if system-tommath is too old

cb391d5

sjaeckel force-pushed the ltm-1.2.0 branch from e15b565 to a7e4a6b Compare May 25, 2020 16:34

XMALLOC etc. were changed to MP_MALLOC etc.

6765075

sjaeckel force-pushed the ltm-1.2.0 branch from a7e4a6b to 6765075 Compare May 25, 2020 16:47

mkj merged commit b4bd23b into mkj:master May 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update LibTomMath to 1.2.0 #84

Update LibTomMath to 1.2.0 #84

sjaeckel commented Oct 25, 2019

mkj commented Nov 5, 2019 •

edited

Loading

mkj commented May 25, 2020

sjaeckel commented May 25, 2020

themiron commented Jun 18, 2020

mkj commented Jun 18, 2020 •

edited

Loading

themiron commented Jun 18, 2020

sjaeckel commented Jun 18, 2020

minad commented Jun 18, 2020

minad commented Jun 18, 2020

themiron commented Jun 19, 2020 •

edited

Loading

mkj commented Jun 25, 2020

sjaeckel commented Jun 25, 2020

czurnieden commented Jun 26, 2020

Update LibTomMath to 1.2.0 #84

Update LibTomMath to 1.2.0 #84

Conversation

sjaeckel commented Oct 25, 2019

mkj commented Nov 5, 2019 • edited Loading

mkj commented May 25, 2020

sjaeckel commented May 25, 2020

themiron commented Jun 18, 2020

mkj commented Jun 18, 2020 • edited Loading

themiron commented Jun 18, 2020

sjaeckel commented Jun 18, 2020

minad commented Jun 18, 2020

minad commented Jun 18, 2020

themiron commented Jun 19, 2020 • edited Loading

mkj commented Jun 25, 2020

sjaeckel commented Jun 25, 2020

czurnieden commented Jun 26, 2020

mkj commented Nov 5, 2019 •

edited

Loading

mkj commented Jun 18, 2020 •

edited

Loading

themiron commented Jun 19, 2020 •

edited

Loading