< bitcoin-git> [bitcoin] dongcarl opened pull request #14741: Indicate -rpcauth option password hashing alg (master...2018-11-improve-rpcauth-help) https://github.com/bitcoin/bitcoin/pull/14741
< meshcollider> Ooh GitHub now has force-push notifications
< meshcollider> That's useful
< meshcollider> See e.g. #14733
< gribble> https://github.com/bitcoin/bitcoin/issues/14733 | p2p: allow p2ptimeout to be configurable, speed up slow test by zallarak · Pull Request #14733 · bitcoin/bitcoin · GitHub
< dongcarl> Any reason why `bitcoin/share/rpcauth/rpcauth.py` generates a 16 byte salt instead of a 32 byte salt? I believe RFC 2104 recommends the key length to be larger than or equal to the hash output length: https://tools.ietf.org/html/rfc2104#section-3
< dongcarl> It also seems that the format string will only output one character if the randomly generated byte is below 0x10, which I don't think is the expected behavior...
< sipa> dongcarl: you don't need an HMAC here
< sipa> or ratherx you don't actually need a MAC here
< sipa> just a hash with some protection against dictionaries
< sipa> so making the salt large enough to make a dictionary attack infeasible is enough
< sipa> 64 bits probably suffices
< dongcarl> sipa: right, but it wouldn't hurt to have a few more bits of entropy and conform to the spec?
< dongcarl> Also with the format string bug we have less than 64 bits at times
< sipa> dongcarl: link to code?
< dongcarl> That format string will only output 1 character if the r in salt_sequence is less than 0x10
< sipa> nice catch
< sipa> but it's not a problem; the set of encoded salts still has 128 bits of entropy
< sipa> oh, no!
< sipa> slightly less
< dongcarl> Right
< sipa> there are collisions
< dongcarl> I'll PR to fix it to make it according to normal expectations
< sipa> dongcarl: i don't think "comforming to the spec" matters here; the spec is to satsify the requirements of a secure MAaC; we don't need a secure MAC
< sipa> but yes, please fix the formatting issue
< dongcarl> Okay gotcha
< jarthur> Wait, why slightly less than 128? If r is more than 0x10, don't we get more than 1 character?
< sipa> the number of characters doesn't matter
< sipa> the number of combinations matters
< sipa> but given that there are distinct inputs that map to the same salt string (e.g. (0x1, 0x23, 0x45) and (0x12, 0x34, 0x5) both map to 12345)
< jarthur> Makes sense now. Thanks :)
< jarthur> The format line could just be replaced by using binascii.hexlify, that way we'll get two-digit hexes as ASCII bytes and won't need the later UTF-8 encodes. Would just need to use salt.decode('ascii') in the printed string.
< dongcarl> jarthur: I just did os.random(16).hex()
< dongcarl> works fine
< jarthur> I like that even better
< jarthur> nice
< bitcoin-git> [bitcoin] dongcarl opened pull request #14742: Properly generate salt in rpcauth.py (master...2018-11-fix-rpcauth-salt) https://github.com/bitcoin/bitcoin/pull/14742
< sipa> dongcarl: do we require python 3.6?
< dongcarl> sipa: I think we require python at least
< dongcarl> python3
< jarthur> bytes.hex was introduced in Python 3.5
< dongcarl> we require python 3.4 for tests according to https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md
< dongcarl> jarthur: link?
< dongcarl> Ooof
< sipa> because python 3.6 has the "secret" module
< sipa> for generating cryptographic strength randomness
< luke-jr> RHEL seems to be at Python 3.4 (w/ EPEL)
< dongcarl> Okay guess I'll conform to python 3.4...
< dongcarl> :-(
< * luke-jr> guesses he should probably reinstall Py3.4 for this purpose
< jarthur> Too bad. Python 3.6 was one of the best Python releases to date, features-wise.
< dongcarl> Yeah I think 3.6 was optional types as well?
< luke-jr> jarthur: we'll get there eventually
< luke-jr> it tends to be RHEL holding things back, and RHEL 8 is supposedly due this year
< Dizzle> Cool, I'm not worried, just wishful
< luke-jr> although Debian stable is only 3.5
< luke-jr> and next not due until 2019
< jarthur> dongcarl: yea, though I'm kind of lumping 3.5 and 3.6 together since the latter polished a lot of the 3.5 stuff. f-strings, async iteration and generation, optional typing, efficient dicts, path-like objects
< meshcollider> sipa: if it's secret, how do you know about it
< dongcarl> meshcollider: the sipa knows all, don't you know? https://cryptopop.files.wordpress.com/2017/12/pieter_wuille_v2.png?w=768
< luke-jr> XD
< luke-jr> dongcarl: is he a gardener?
< dongcarl> Gunna stop this digression from development discussion here, but luke-jr here's the source: https://twitter.com/coindesk/status/947699170353340416
< bitcoin-git> [bitcoin] LucianaMarques opened pull request #14744: [Docs] Added NSIS install steps to build-windows.md (master...windows-doc) https://github.com/bitcoin/bitcoin/pull/14744
< bitcoin-git> [bitcoin] IPGlider opened pull request #14746: Check for invalid prefixes in config file (master...check_invalid_prefixes_in_config_file) https://github.com/bitcoin/bitcoin/pull/14746
< bitcoin-git> [bitcoin] IPGlider closed pull request #14746: Check for invalid prefixes in config file (master...check_invalid_prefixes_in_config_file) https://github.com/bitcoin/bitcoin/pull/14746
< bitcoin-git> [bitcoin] Empact opened pull request #14747: build: Drop AC_CHECK_DECLS for unused __builtin_clz (master...builtin-clz) https://github.com/bitcoin/bitcoin/pull/14747
< dongcarl> Would it be possible to block a user from bitcoin/bitcoin? This guy's been spamming: https://github.com/bitcoin/bitcoin/pull/14742#issuecomment-439635586
< bitcoin-git> [bitcoin] vim88 opened pull request #14749: Refactor: Changes postincrement to preincrement in for loop in src/consensus files (master...postincrement_to_preincrement_src_consensus) https://github.com/bitcoin/bitcoin/pull/14749
< bitcoin-git> [bitcoin] vim88 closed pull request #14749: Refactor: Changes postincrement to preincrement in for loop in src/consensus files (master...postincrement_to_preincrement_src_consensus) https://github.com/bitcoin/bitcoin/pull/14749
< zallarak> Thank you to those that left feedback on my PR. Does anyone else have thoughts? (https://github.com/bitcoin/bitcoin/pull/14733)