< bitcoin-git>
[bitcoin] fanquake opened pull request #21135: scripts: check for control flow instrumentation in security-check.py (master...security_check_cf_protection) https://github.com/bitcoin/bitcoin/pull/21135
< wumpus>
what is it with spaces before and after PR/issue titles 🙂
< vasild>
the i2p "destination" contains the private key for the .b32.i2p address that is derived from it. So, if somebody has yours they can pretend to be your address.
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #21139: doc: Add missing copyright header to util/getuniquepath (master...2102-c) https://github.com/bitcoin/bitcoin/pull/21139
< vasild>
I think it is unwise for i2pd to print it in plaintext in i2pd logs
< wumpus>
yes, although logs can be assumed to be somewhat sensitive, logging private keys to logs is not a good idea in general
< vasild>
sdaftuar: if the request "DEST GENERATE SIGNATURE_TYPE=EdDSA_SHA512_Ed25519" generates "runtime exception: stoi" inside i2pd, then I guess it has a bug or too old and that signature type is not supported (but still should not cause an exception)
< vasild>
from https://geti2p.net/en/docs/api/samv3 "The SIGNATURE_TYPE value may be any name (e.g. ECDSA_SHA256_P256, case insensitive) or number (e.g. 1)"
< wumpus>
fanquake: seems easier than i thought, it looks like symbols in .text start with "f3 0f 1e fa"
< fanquake>
wumpus: heh
< fanquake>
Also the short answer to your Q is no. As when we compile our dummy prog for our test, even when opting out of -fcf-protection, you still end up with endbr64 instructions from libc etc. I’ll elaborate in the PR
< wumpus>
fanquake: if only a few specific symbols we could ignore those
< fanquake>
I think it was 7. Will have another look
< wumpus>
i see, the startup symbols (which come from some .o file) still have the instruction when compiling with -fcf-protection=none
< fanquake>
Annoyingly, when you filter the objdump output with --disassemble=<symbol> that only specifies the start point, at <sym>, but it displays all symbols after. Although you could then cut off the output after the first newline.
< wumpus>
yea i'm going to try to do this without objdump
< wumpus>
the symbol listing part is in pixie, as well as extracting section contents, and i think this doesn't need a full disassembler
< wumpus>
i agree it's annoying though, the obvious 'look at the first instruction at the entry point' doesn't work because that one always has a endbr64
< wumpus>
the straightforward way would be to count the number of occurences of endbr64, if it's more than some threshold it's adding it for every function, otherwise not, but that does seem brittle
< fanquake>
yes, however that is also complicated across all the different binaries. We need to test in the PE bins as well.
< wumpus>
the obstacle with using symbols is that we don't currently look at the normal (debug) symbol table at all in pixie, only the dynamic symbol table, which doesn't contain any exported symbols for a binary normally
< wumpus>
yes, I'm focusing only on ELF for now
< fanquake>
My understanding is that this "should" be supported for macOS too, however (haven't checked thoroughly), my assumption is that our LLVM isn't new enough.
< wumpus>
though a general function 'give me text section contents and I'll say yes or no' should work for the other binary formats too
< wumpus>
if symbols are involved it gets nastier
< fanquake>
Sure. By different binaries I also meant bitcoind, bitcoin-wallet, bitcoin-util etc. As any sort of counting thresholds will be different
< wumpus>
I'm not sure about that, if it's the entry (.o) objects those are the same for all binaries
< fanquake>
ah right. Just checking that the endbr64 count is > that the entry objects count
< wumpus>
"the stuff that executes before main" and is always added by gcc
< wumpus>
right
< wumpus>
there might be a more subtle way though, still looking
< wumpus>
on my system a minimal binary has 6 endbr64 instructions with and 5 endbr64 without fcf
< wumpus>
(in the .text section)
< wumpus>
bitcoind has 39360 and bitcoin-cli has 2665 (with fcf, will try without now)
< fanquake>
If I build a "return 0" c prog on my ubuntu box. With it has 9, without it has 7
< wumpus>
okay
< fanquake>
Actually, sorry, I see the same as you. 5 & 6.
< fanquake>
Wasn't filtering by --section=.text
< wumpus>
phew :)
< fanquake>
heh
< fanquake>
Of the symbols in the text section: _start, __do_global_dtors_aux, frame_dummy, __libc_csu_init, __libc_csu_fini have an endbr64 instruction. deregister_tm_clones & register_tm_clones don't. Then we control <main>
< wumpus>
yes-looking at the main symbol might be the obvious thing
< wumpus>
we can assume it to be always present, after all, and it beats counting
< wumpus>
the only snag is that in that case stripping symbols will break the check (as there will be nothing left to point to main), but we don't do that anyhow for our release binaries
< fanquake>
I thought we did?
< wumpus>
okay in that case maybe i can come up with a heuristic to find main
< wumpus>
you're right.
< wumpus>
but before or after the security check?
< fanquake>
hmm good call
< fanquake>
I think it might be after, as the security & symbols checks are run straight after make
< wumpus>
i only test fcf level full and none, i think distinguishing between the levels is possible but much more involved (e.g. in addition to function calls you'd also have to check branches)
< wumpus>
but it's enough to protect against accidentally leaving it disabled or compiling with a compiler without fcf support
< fanquake>
Yes I think that's fine. I'll drop my ELF change for yours, and then just sort out PE.
< promag>
b10c: hi
< wumpus>
maybe we can do a similar thing with PE by just parsing the disassembly (e.g. find main) dunno, i don't feel like writing a PE parser right now 😒
< wumpus>
or simpler, you said that objdump will return disassembly *from* a certain symbol, so if you check the first instruction from there
< fanquake>
Yes. The "from" includes 5-6 lines of prelude, before the actual symbol you want, but we should be able to skip that, find the line with <main>, then check that the one after has the instruction
< wumpus>
right !
< bitcoin-git>
[bitcoin] maayank opened pull request #21141: Add new format string placeholders for walletnotify (master...walletnotify-blockhash) https://github.com/bitcoin/bitcoin/pull/21141
< pinheadmz>
did 0.9 retro-actively prune unspendable outputs that were already in the UTXO set? Or are we stuck with some early OPRETURNs in there, forever?
< pinheadmz>
oh maybe just found my answer in txdb.cpp: CCoinsViewDB::Upgrade() there is a migration
< sipa>
indeed
< pinheadmz>
that wasn't actually rolled out until 0.15 though to go with the per-tx out update? So some old opreturns hung around in the set for a few versions
< sipa>
we had migration code for 0.7 to 0.8 too, but that may not be in the codebase anymore
< bitcoin-git>
bitcoin/master 166266a Hennadii Stepanov: script: Make LXC container size suitable for gitian builds
< bitcoin-git>
bitcoin/master deb185d Wladimir J. van der Laan: Merge #21130: script: Make LXC container size suitable for gitian builds
< bitcoin-git>
[bitcoin] laanwj merged pull request #21130: script: Make LXC container size suitable for gitian builds (master...210209-lxc) https://github.com/bitcoin/bitcoin/pull/21130
< sdaftuar>
vasild: thanks. i tried "DEST GENERATE SIGNATURE_TYPE=EdDSA_SHA512_Ed25519" and got the "runtime exception: stoi" inside i2pd, but "DEST GENERATE SIGNATURE_TYPE=7" indeed seems to work
< sdaftuar>
fyi i was able to install the latest release of i2pd on my mac, and the PR adding i2p support seems to run fine there (though i haven't yet connected to other i2p peers yet)
< sdaftuar>
ah and i just build i2pd from source, and that works too. sweet!
< jonatack>
nice!
< sdaftuar>
hah, if i understand the i2pd bug correctly, releases 2.24.0 and later seem to have this fixed, and i was on 2.23.0
< jonatack>
we're up to 4 I2P peers now, woot
< phantomcircuit>
can i use std::filesystem?
< phantomcircuit>
i mean like it works on my system but is that allowed
< sipa>
phantomcircuit: i believe it does not work for all our currently supported platforms
< sipa>
but look at fs.h, which has an abstraction that closely matches it
< phantomcircuit>
yeah im using that now, it's missing the ::exists method which would simply my code a bit
< phantomcircuit>
i guess i can just use the boost filesystem stuff directly
< fjahr>
#19145 might be ready for merge. It has 4 ACKs by sjors, jonatack, ryanofsky and achow101.