--- Log opened Sat Jan 11 00:00:03 2020
00:42 -!- midnight [~midnightm@unaffiliated/midnightmagic] has quit [Ping timeout: 264 seconds]
00:57 -!- ddustin [~ddustin@unaffiliated/ddustin] has joined #secp256k1
01:02 -!- ddustin [~ddustin@unaffiliated/ddustin] has quit [Ping timeout: 260 seconds]
02:37 < real_or_random> fwiw, I'm in favor of constant-time code for special cases/errors because it's typically not a loss of performance (except for a CMOV maybe, yes).
02:39 < real_or_random> anyway we don't optimize for the unusual executions paths, so this is a case in which we get constant-time code basically for free. so we should do it, in particular because it's hard to tell how some function will be used later
02:40 < real_or_random> this is true for both our internal low-level functions (which are written with a specific use in mind but are later maybe used for something else too) and for public API functions (where we have no control what people will do)
02:44 < real_or_random> ecdsa loop: can't we just get rid of that loop?
03:12 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Ping timeout: 260 seconds]
03:45 -!- belcher [~belcher@unaffiliated/belcher] has joined #secp256k1
04:12 < gmaxwell> real_or_random: and what, just fail to sign (with low probablity?)
04:13 < real_or_random> yep
04:15 < gmaxwell> maybe but I worry that callers written by people who can't reason about 1:2^256 will shoot themselves in to foot trying to implement htat loop themselves.
04:21 < gmaxwell> essentially, having the loop is cheap insurance against bothering people with OCD. Or at least it was cheap insurance until it broke automated constant time analysis. :)
04:22 < gmaxwell> FWIW, this valgrind approach is really good, I can't find a way to bypass it (other than using valgrind macros, of course).
04:28 < real_or_random> OCD?
04:29 < gmaxwell> obsessive.
04:30 < gmaxwell> of the sort, "I better not switch off of php-secp256k1-- that libsecp256k1 can fail! I know its not likely but I won't deploy broken code!"
04:31 < real_or_random> https://www.azquotes.com/quote/847287
04:31 < real_or_random> sorry
04:32 < gmaxwell> yeah, well we write software for humans to use it. In any case, I'm not arguing strongly. I think the benefit of not causing idiots-in-a-hurry to worry is worth something-- worth the code to have a loop, for example.  It might not be worth breaking a powerful constant time analysis test.
04:33 < gmaxwell> real_or_random: if you notice my latest PR just eliminated the loops for the context randomization.
04:33 < gmaxwell> but in that case, it could never cause a fault visible to the user.
04:34 < real_or_random> I agree in general. that dijkstra attitude is right but not what we want for this piece of code
04:35 < real_or_random> btw, just 12 hours ago: https://github.com/bitcoin-core/secp256k1/pull/701#discussion_r365477162
04:36 < real_or_random> I guess breaking the valgrind test is a much more tangible drawback than the worry that someone will add a loop
04:36 < gmaxwell> it's not totally crazy for people to worry about stuff like this:  if it can go wrong in a rare case, maybe it can go wrong in a case that attacker could trigger. I'm not saying thats the case here, but people would generally be right to worry about it.
04:37 < real_or_random> sure
04:37 < gmaxwell> Yeah, now that it breaks a useful test I think there might be some merit to eliminating the loop. Though we do have other alternatives to fix valgrind.
04:37 < real_or_random> (and these cases can also matter for static analysis)
04:37 < gmaxwell> Yes, these would also break any kind of static constant time analysis.
04:38 < gmaxwell> If valgrind were actually making a mistake I would be much more inclined to address it with a suppression, but it's not-- this isn't constant time.
04:38 < gmaxwell> though an alternative would be to just add the valgrind instrumentation.. and presumably any other static analysis would also provide an unsecret() call.
04:40 < gmaxwell> E.g. we'd just add a function secp256k1_unsecret(ctx,void *,len)  and when the context is created with an unsecret flag, it'll call valgrind_make_mem_defined on its argument.  In some other static analyis thing, you'd replace the implementation of libsecp256k1_unsecret.
04:41 < real_or_random> by the way, "unsecret" is typically called "declassify" https://en.wikipedia.org/wiki/Information_flow_(information_theory)#Declassification
04:42 < gmaxwell> the only reason I didn't just do that yet is sipa seemed squicked out by adding the valgrind instrumentation to the production code.  But I read what it inserts and I don't feel at all bothered by it.
04:42 < gmaxwell> oh, nice, declassify.
04:44 < gmaxwell> the declassify approach has an additional value of documenting the non-constant-time behavior clearly.
04:44 < gmaxwell> my preference is (1) make the secret handling functions constant time to the greatest extent possible at low cost, and whatever remains that is harmless, address with a declassify operation.
04:45 < real_or_random> +1
04:47 < sipa> ack
04:48 < sipa> let's go with that approach
04:48 < gmaxwell> \O/
04:50 < sipa> gmaxwell: how does the cost of valgrind mark initialized compare to the overhead of calling seco256k1_unclassify function that needs to inspect the context?
04:50 < real_or_random> I didn't expect the valgrind approach to be that powerful
04:52 < gmaxwell> sipa: it's probably lower, however, I don't want to diminish the power of valgrind when not using it for CT checking.
04:53 < gmaxwell> the problem is that valgrind doesn't distinguish secret from uninitilized... so we'd end up hiding other errors. Also, if someone was paranoid about the valgrind ops breaking things, thats solved by not calling them.
04:54 < gmaxwell> this is not in any performance critical loop in any case.
04:54 < sipa> ah, good point
05:31 < gmaxwell> ==892007== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
05:32 < sipa> \o/
05:36 < real_or_random> nice
05:37 < gmaxwell> I haven't done the tweaks yet. :)
05:37 < gmaxwell> but this is clean for pubkey/signing/ecdsa.
05:38 < gmaxwell> https://github.com/bitcoin-core/secp256k1/pull/710/commits/fcdf1080fec4c0756ccd5310d789209a4eaa7d72
05:42 < gmaxwell> probably needs some more comments and such... e.g. so there isn't a mysterious and unexplained DECLASSIFY flag in the public header.
05:42 < gmaxwell> I'm also not sure if I should also have it declassify the final output of signing and pubkey... technically they are declassifyable, but since their output is spat straight out, the caller can do it.
05:44 < gmaxwell> Valgrind doesn't currently have a magic operation to check if memory is classified that returns a flag without making valgrind print an error, but I'm going to try to add one to valgrind upstream so I can make the test check that e.g. the ECDH output is classified.
05:45 < gmaxwell> (there is a check, but it makes valgrind print an error)
05:55 < gmaxwell> should context creation fail if called with the declassify flag but declassification support isn't compiled in? (e.g. built without memcheck header)?
06:32 < real_or_random> https://github.com/bitcoin-core/secp256k1/pull/711
10:54 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has quit [Ping timeout: 240 seconds]
11:01 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has joined #secp256k1
11:22 -!- GAit [~GAit@unaffiliated/gait] has quit [Ping timeout: 265 seconds]
11:26 < gmaxwell> if anyone feels like doing it, this valgrind constant time check should get run on * platforms. particularly testing with/without asm and 32/64 bit.
11:29 < gmaxwell> ah, found another on, on power9
11:30 < gmaxwell>         sign = 2 * (u_last > 0) - 1;
11:30 < gmaxwell> src/ecmult_const_impl.h:111
11:37 < gmaxwell> whoo boy, 32bit scalar code lights up like a christmas tree (at least on power9)
11:45 < gmaxwell>    0x00000000040c41e8 <+3176>:	rldicl  r7,r7,1,63
11:45 < gmaxwell> 444	    muladd(m12, SECP256K1_N_C_0);
11:45 < gmaxwell> => 0x00000000040c41ec <+3180>:	bge     0x40c41f8 <secp256k1_scalar_reduce_512+3192>
11:45 < gmaxwell>    0x00000000040c41f0 <+3184>:	addi    r10,r31,1
11:45 < gmaxwell> and yeah, it's emitting branches.
11:45 < gmaxwell> (bge is a branch on greater or equal)
11:48 -!- GAit [~GAit@unaffiliated/gait] has joined #secp256k1
12:11 -!- lukedashjr [~luke-jr@unaffiliated/luke-jr] has joined #secp256k1
12:12 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Ping timeout: 268 seconds]
12:16 -!- lukedashjr is now known as luke-jr
12:32 -!- real_or_random [~real_or_r@2a02:c207:3002:7468::1] has quit [Ping timeout: 260 seconds]
12:33 -!- afk11 [~afk11@gateway/tor-sasl/afk11] has quit [Ping timeout: 240 seconds]
12:35 -!- afk11 [~afk11@gateway/tor-sasl/afk11] has joined #secp256k1
12:38 -!- real_or_random [~real_or_r@173.249.7.254] has joined #secp256k1
14:24 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined #secp256k1
15:36 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has joined #secp256k1
15:43 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Ping timeout: 265 seconds]
15:44 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has joined #secp256k1
17:17 -!- belcher [~belcher@unaffiliated/belcher] has quit [Quit: Leaving]
19:31 -!- midnightmagic is now known as midnight
20:36 -!- cfields_ [~cfields@unaffiliated/cfields] has quit [Remote host closed the connection]
20:37 -!- cfields [~cfields@unaffiliated/cfields] has joined #secp256k1
20:38 -!- kanzure [~kanzure@unaffiliated/kanzure] has quit [Ping timeout: 245 seconds]
20:38 -!- zmanian_ [sid113594@gateway/web/irccloud.com/x-buuyobkxouaphigw] has quit [Ping timeout: 245 seconds]
20:39 -!- kanzure [~kanzure@unaffiliated/kanzure] has joined #secp256k1
20:40 -!- zmanian_ [sid113594@gateway/web/irccloud.com/x-wrpewajemeemlhgf] has joined #secp256k1
20:45 -!- afk11 [~afk11@gateway/tor-sasl/afk11] has quit [Remote host closed the connection]
20:45 -!- afk11 [~afk11@gateway/tor-sasl/afk11] has joined #secp256k1
--- Log closed Sun Jan 12 00:00:04 2020