< phantomcircuit>
sipa, i hadn't i've been otherwise occupied
< phantomcircuit>
sipa, yeah that was going to be my adjustment, add a parameter that sets the runtime in wall clock time, (0 being forever)
< phantomcircuit>
also realized it should be displaying the output from each subprocess before they finish but that gets really messy so i was thinking about how to handle that
< sipa>
right
< phantomcircuit>
afl just writes output from the subprocesses to logfiles which i guess is an option but also kind of hackey
< phantomcircuit>
i dont really have a better idea atm though
< phantomcircuit>
sipa, any ideas?
< sipa>
just send them to stderr?
< sipa>
or do you need to intercept them from python somehow?
< phantomcircuit>
the python script can read the output continuously and then write to whatever
< phantomcircuit>
writing directly to stderr is really confusing though cause none of the debug lines have any context
< sipa>
ah you want to add a prefix to indicate which fuzzer it's from?
< phantomcircuit>
i could write the PID but for long runs they won't be unique
< phantomcircuit>
i guess i could just assign each fuzzing process a longer than pid random id
< sipa>
i don't understand what the problem is or what you're trying to do
< phantomcircuit>
sipa, it's just really confusing to have all the debug output from the fuzzers writing to stderr interleaved, but also it's not helpful to have it all buffered until the process exits
< phantomcircuit>
i guess that's pretty nitpickey
< sipa>
well give each some incrementally assigned id + description string (with the fuzz target name + config, if any part of it is variable)
< jonasschnelli>
achow101, sipa: for the 0.21.0rc4 mac signatures,... does it matter if I use the apple codesign_allocate tool for the signature? AFAIK that worked with sipas fix.
< sipa>
jonasschnelli: as far as we know, our (now patched) cctools codesign_allocate tool works identical to Apple's
< bitcoin-git>
bitcoin/master fa749fb MarcoFalke: rpc: Replace boost::variant with std::variant for RPCArg.m_fallback
< bitcoin-git>
bitcoin/master bc8ada1 MarcoFalke: Merge #20736: rpc: Replace boost::variant with std::variant for RPCArg.m_f...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #20736: rpc: Replace boost::variant with std::variant for RPCArg.m_fallback (master...2012-rpcStdVariant) https://github.com/bitcoin/bitcoin/pull/20736
< jonasschnelli>
sipa: we have to clear the depende cache to make your patch work,.. right? Getting an invalid signature
< jonasschnelli>
(but I have the same hash as the others,... maybe no-one cleared the cache?)
< jonasschnelli>
trying with empty cache
< jonasschnelli>
no.. it was't the cache
< jonasschnelli>
getting invalid macOS signature
< jonasschnelli>
What I did:
< jonasschnelli>
- Built 0.21.0rc4 gitian macOS
< jonasschnelli>
- Verified bitcoin-0.21.0rc4-osx-unsigned.tar.gz, got the correct 10ae57ee735cd2a894a75165082c384769da3b634eca9346b1d20585e1edfd0c
< jonasschnelli>
Created the signature: ./detached-sig-create.sh -s "Bitcoin"
< jonasschnelli>
used the signature to gbuild the signer: ./bin/gbuild --num-make 6 -j11 --memory 5000 --url bitcoin=/home/jonasschnelli/gitian/bitcoin --commit signature=v0.21.0rc4
< jonasschnelli>
(I upgraded to macOS 11 and updated XCode since I tested sipas fix on master)
< sipa>
oh
< sipa>
so things may have changed again...
< sipa>
can you use achow"s signer and patched-cctools' codesign_allocate instead?
< jonasschnelli>
I try that later on
< jonasschnelli>
I'm not sure if the upgrade to macOS 11 did cause this)
< bitcoin-git>
[bitcoin] hebasto closed pull request #20824: build: Do not set _FORTIFY_SOURCE if it causes compiler warnings (master...210101-fortify) https://github.com/bitcoin/bitcoin/pull/20824
< sipa>
jonasschnelli: well, it seems likely
< sipa>
maybe they "fixed" the issue we encountered earlier...
< jonasschnelli>
That would mean rc3 would sign valid
< sipa>
right
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20844: test: Add sanitizer suppressions for AMD EPYC CPUs (master...2101-testEpycSan) https://github.com/bitcoin/bitcoin/pull/20844
< jonasschnelli>
signing rc3 gives me also an invalid signature
< jonasschnelli>
(apple didn't fix the codesign_allocate difference in 11.16 then)
< jonasschnelli>
achow101: can you also try to sign rc4 with your dummy certificate? Just to see if the error is on my end.
< jonasschnelli>
achow101, sipa: still getting invalid signatures. I tried using achow101't signapple tool, which worked locally, but attaching rc4 sig does not work (invalid sig). I guess this is due to sipas patch?
< jonasschnelli>
I'm also getting an invalid signature (rc4) when using achow101 tool, then apply the signature with the codesign_allocate binary from rc3.
< MarcoFalke>
jonasschnelli: Can you sign rc3 with the achow101 tool?
< MarcoFalke>
rc3 doesn't have the sipa patch
< jonasschnelli>
MarcoFalke. Let em try
< jonasschnelli>
MarcoFalke: yes. Signing rc3 works with achow101 tool.
< MarcoFalke>
So let's just rever the sipa patch for rc5?
< jonasschnelli>
I think so. Though switching to an new signing tool between rc's is not ideal but I don't see a different solution
< jonasschnelli>
I'll try rc4 again... but I'm pretty sure I haven't made a mistake
< MarcoFalke>
you could downgrade your mac? ;)
< jonasschnelli>
I could,... or use a VM of my older state.
< jonasschnelli>
But somehow I don't think it is the reason
< jonasschnelli>
I try now on macOS 10.12.2
< jonasschnelli>
(a VM is have ready)
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20845: net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers (master...2101-netLogDisconnect) https://github.com/bitcoin/bitcoin/pull/20845
< jonasschnelli>
sipa: I used a macOS 10.12 (older) VM and signed the 0.21.0rc4 there. Getting also an invalid signature after attaching the sig back in gitian.
< bitcoin-git>
[bitcoin] theStack opened pull request #20848: Add gitian PGP key for theStack (master...add_gitian_pgp_key_for_theStack) https://github.com/bitcoin/bitcoin/pull/20848
< wumpus>
oh noo not more macos signing issues 😢
< jonasschnelli>
wumpus: sadly,... but I tripple checked everything. I guess sipas patch does not work for rc4 (it did for master a while ago when I tested it)
< jonasschnelli>
I'd like achow101 and sipa to confirm that its not working and we might need to switch to achow101 signapple tool for rc5 (revert sipas codesign_allocate patch)
< MarcoFalke>
Could you also check if rc1 on the 0.19 and 0.20 branch work
< MarcoFalke>
I think they haven't been released either
< wumpus>
jonasschnelli: if using achow101's tool makes the process more reliable I'm all for that tbh
< wumpus>
it feels more like gambling than engineering at this point :)
< wumpus>
MarcoFalke: yea I think it makes sense to be sure we can do this reliably, then move on with the other releaes, instead of spreading out the experimentation over multiple branches
< MarcoFalke>
if it works, we can close #20738 and #20739
< wumpus>
(there's more in there than the macos signing)
< MarcoFalke>
only some minor doc fixups in the 0.20 one
< BlueMatt>
is it expected that new versions of core (0.20.1, I guess?) will send tx invs even if you set fRelay to false in the version message and never send a filter?
< BlueMatt>
I suddenly see a *ton* of nodes violating fRelay
< jonatack>
BlueMatt: hm, iirc no except bloom filterload/clear
< bitcoin-git>
[bitcoin] vasild opened pull request #20849: net: disconnect peers by address without using a subnet (master...disconnect_by_subnet_fix) https://github.com/bitcoin/bitcoin/pull/20849
< wumpus>
it's a bit absurd, we really shouldn't have waited that long to do rc4
< MarcoFalke>
it slipped the pre-holiday window. If the release doesn't go out before Dec 14th, it is best to wait till January
< wumpus>
would have found this new signing issue so much faster
< MarcoFalke>
Ok, that is true
< vasild>
jonatack: when I extended CNetAddr to support longer-than-16-bytes addresses (as part of bip155) I also changed CSubNet to only handle ipv4 and ipv6 addresses - because it contains char netmask[16]; it makes no sense to apply it to e.g. 32 byte torv3 address
< jonatack>
If help docs are eligible for 0.21.0rc5, #20829 targets 0.21 and has a couple acks
< jonatack>
vasild: yes, thank you, that occurred to me right after checking git blame
< vasild>
In general, subnetting only makes sense for ipv4 and ipv6
< wumpus>
it makes sense for none of the other networks that we support at least
< vasild>
but I failed to verify the callers of the constructor CSubNet(CNetAddr&) which used to create one-host-subnet before and would also "work" for torv2 addresses (in the code before bip155) by matching a single address
< wumpus>
good to catch that before the release
< vasild>
MarcoFalke: found the issue somehow :)
< MarcoFalke>
It is not super critical, but still a nice to have fix
< wumpus>
yes
< achow101>
jonasschnelli: can you upload codesign's result?
< achow101>
given that the signapple result is giving a vmsize of 0x7b000, I think apple fixed codesign_allocate
< vasild>
because banman uses "std::map<CSubNet, CBanEntry>" - it only bans subnets
< vasild>
I am inclined to restore the "nonsensical" subnetting of non-IPv[46] networks by assuming one-host-subnet for those
< vasild>
banman only knows how to ban subnets and also de/serializes to disk CSubNet objects
< achow101>
jonasschnelli: it seems that codesign_allocate has changed back to what we were expecting it to do
< vasild>
ideally banman should have a list of banned subnets (CSubNet) + a list of banned hosts (CNetAddr), but that would mean changing the disk format of the banlist
< sipa>
vasild: is this something we need to fix for 0.21?
< sipa>
i assume so?
< vasild>
yes, I guess so, otherwise we don't know how to ban non-ipv46 addresses
< MarcoFalke>
If the fix is too involved, we can document it as known bug and fix in 0.21.1
< vasild>
allowing CSubNet of non-ip addresses seems like a smaller change than banman disk format
< sipa>
it"s not like banning tor addresses means much, as they can't occur on incoming connections
< vasild>
that's right
< MarcoFalke>
jup, it is even more a "just nice to have" fix
< sipa>
what happens if you try?
< jonasschnelli>
achow101: I also tried with MacOS 10.12 (can upload that as well). I’m afk for 2h,... will upload more later.
< vasild>
banlist.dat + banhosts.dat - have two separate lists and two separate files (not changing the format of banlist.dat), what about that?
< sipa>
vasild: shouldn't be needed
< sipa>
vasild: there was also some desire to support banning by ASN
< sipa>
which will require further changes anyway
< vasild>
sipa: if you try to ban a tor address it would store an invalid CSubNet in banman which matches nothing, e.g. the address will not be banned
< sipa>
ok, not terrible... i'm ok with either
< sipa>
if it's easy to restore csubnet support for other networks, that seems like reasonable (temporary) fix
< sipa>
we can also leave it be
< achow101>
jonasschnelli: I think codesign and codesign_allocate versions aren't tied to the os version
< achow101>
at least codesign_allocate is part of some xcode command line developer tools package
< sipa>
jonasschnelli: i can't explain why signing with applesign doesn't work though, unless you also used codesign_allocate from apple.for thaf
< sipa>
* for that
< wumpus>
vasild: sounds like the most generic solution to me, to define that a 'subnet' for other networks is always a single address, and keep the rest of the code the same
< wumpus>
otherwise you'd have essentially to change all the code using subnets to be CSubnet-or-separate-address
< achow101>
sipa: I think jonasschnelli used the macports codesign_allocate which will now behave differently to our codesign_allocate
< sipa>
gah
< sipa>
wumpus: indeed
< achow101>
sipa, jonasschnelli: If I revert the patch to cctools, I am able to attach the signature and get a valid result
< sipa>
achow101: then why did jonasschnelli' rc3 signing fail too?
< sipa>
still, let's revert my patch
< sipa>
it shouldn't be needed anymore in the longer term anyway with applesign (using matching cctools/macports is probably easiest for codesign_allocate)
< achow101>
sipa: jonasschnelli may have done it incorrectly. Properly getting the detached-sig-apply.sh script to use the right codesign_allocate is slightly tricky
< achow101>
I did it wrong a few times before I figured out I needed to re-pack the tarball with the correct codesign_allocate as the script will unpack it again itself
< achow101>
sipa: I think this problem just highlights the fact that signapple needs to implement this allocation stuff too
< achow101>
but modifying macho bins to insert another load command seems to be non-trivial
< vasild>
"if it's easy to restore csubnet support for other networks, that seems like reasonable (temporary) fix" -- sipa, done in ^ it is maybe not too complicated
< sipa>
vasild: looks reasonable, will review in more detail
< vasild>
ok :)
< * vasild>
afk
< achow101>
I think if we are going to use signapple instead of codesign, then we should build codesign_allocate for mac and include it in the tarball. That way we don't rely on the signer to remember to use macports
< sipa>
achow101: yeah
< sipa>
good idea
< achow101>
at least until I implement allocation
< jonasschnelli>
<achow101> sipa: jonasschnelli may have done it incorrectly. Properly getting the detached-sig-apply.sh script to use the right codesign_allocate is slightly tricky
< jonasschnelli>
^ the codesign_allocate is part of the bitcoin-0.21.0rc3-osx-unsigned.tar.gz? right?
< jonasschnelli>
the detached-sig-apply.sh takes the binary in bitcoin-0.21.0rc3-osx-unsigned.tar.gz, right?
< sipa>
hmm yes, i would expect it to
< jonasschnelli>
As for rc4: i tried on macOS 11, verified the hash of bitcoin-0.21.0rc4-osx-unsigned.tar.gz (matched the others), verified the patched codesign_allocate, signed locally, verified the signed app (ok), gbuilt the signer with the created signature (failed)
< jonasschnelli>
I also tried by signing on a macOS 10.12 (relative old) VM. Same result.
< jonasschnelli>
(codesign was already installed, I think that binary is not part of XCode)
< jonasschnelli>
[18:19:06] <sipa> jonasschnelli: i can't explain why signing with applesign doesn't work though, unless you also used codesign_allocate from apple.for thaf
< jonasschnelli>
^ achow101 applesign tool only worked on rc3 AFAIK
< jonasschnelli>
<achow101> I think if we are going to use signapple instead of codesign, then we should build codesign_allocate for mac and include it in the tarball. That way we don't rely on the signer to remember to use macports
< jonasschnelli>
^ absolutely agree. Otherwise we'd likely end up again with a invalid signature. Another option would be to verify the signature before committing (or at least refuse to finalise the signed dmg if it is invalid).
< achow101>
jonasschnelli: did you repack the tarball after changing codesign_allocate?
< jonasschnelli>
achow101: rc3 has the unpatched codesign_allocate,.. right?
< achow101>
yes
< jonasschnelli>
achow101: I have tested rc4 with a manually copied codesign_allocate from rc3 (manually copy during gbuild)
< achow101>
how did you manually copy it?
< achow101>
I tested by just running the apply script locally without gitian
< jonasschnelli>
fiddled with the gitian .yml
< achow101>
ah
< achow101>
so detached-sig-apply.sh will untar the unsigned.tar.gz again and use the codesign_allocate it finds when it does that
< jonasschnelli>
I added a step to copy/overwrite the codesign_allocate taken from inputs/ in the yml
< sipa>
this all sounds very error prone, so hard to draw any conclusions
< achow101>
so gitian-osx-signer.yml will untar it. then it runs detached-sig-apply.sh which untars it again into another dir and uses the stuff from that second one
< MarcoFalke>
did someone check rc1 on 0.19 and 0.20 ?
< jonasschnelli>
Creating the signature for rc4 is not very error prone
< MarcoFalke>
Just to make sure and not run into issues there as well
< jonasschnelli>
MarcoFalke: yes. We should also test those.
< sipa>
jonasschnelli: because this is all presumably due to alignment issues, there is a 50% chance that a given binary will just work, regardless of what approach is used
< jonasschnelli>
I do a testrun with 0.19.2rc1
< jonasschnelli>
sipa: I also have the feeling its that
< sipa>
so we can't conclude that something will work because it has worked for another version in the past (absent better inspection of course0
< achow101>
jonasschnelli: my point is that your change to the gitian .yml doesn't work because it doesn't change codesign_allocate for the second untar that the apply script does
< achow101>
sipa: we do know that macOS codesign_allocate has reverted back to rounding to 0x1000 since the vmsize on rc4 is 0x7b000
< sipa>
achow101: ah ok
< jonasschnelli>
achow101: I guess you're right (my gitian [not]fix).
< achow101>
jonasschnelli: yeah, it's very unintuitive. I made the same mistake several times when trying to test that
< jonasschnelli>
So... I'll try again to test rc4 without sipas patch (with apples codesign)
< jonasschnelli>
I might need to rebundle the tar file with the rc3 codesign_allocate
< achow101>
you have to
< jonasschnelli>
or set the CODESIGN_ALLOCATE env in gitian
< jonasschnelli>
(would that work achow101?)
< jonasschnelli>
since detached-sig-apply.sh L34
< achow101>
that might work
< achow101>
I hadn't considered that
< jonasschnelli>
will do that test asap (tomorrow)
< jonasschnelli>
achow101, sipa: rc4 without sipas patch works (with native apple codesign)
< jonasschnelli>
I guess the whole fixing was a unnecessary roundtrip
< achow101>
at least it tells us we can't trust apple
< jonasschnelli>
heh... indeed
< sipa>
ok great to hear
< jonasschnelli>
0.19.2rc1 works also (with apples macOS 11 codesign)
< jonasschnelli>
If there would just be a way to check the version of apples codesign tool
< jonasschnelli>
however,... it got fixed by apple... suckers!
< achow101>
too bad they don't seem to put their version numbers anywhere
< jonasschnelli>
MarcoFalke: there is nothing open for 0.19.2. Is there even need for a rc2?
< jonasschnelli>
achow101: I pushed the 0.19.2rc1 sig (your turn for windows and tagging)
< wumpus>
oh so this was one of those cases where 'just wait' was the correct solution, but we had no idea whether Apple saw it as a problem at all
< wumpus>
let alone urgent enough to fix so quickly
< sipa>
maybe some of our twitter ramblings got noticed :)
< sipa>
or maybe they knew about it since a long time, and it only now got addressed
< wumpus>
right you never know who was paying attention
< luke-jr>
wumpus: thoughts on release notes encouraging upgrading sooner rather than later to demonstrate a minimum lower bound on upgrade time for Taproot?
< luke-jr>
ie, "upgrade asap to help prove we can deploy Taproot faster safely"
< luke-jr>
(not limited to wumpus responding :P)
< wumpus>
luke-jr: no problem with that, but i'm not convinced that will give very useful information, how comparable is updating to a major version versus updating to a softfork
< wumpus>
jonasschnelli: sure, we could tag 0.19.2 final, though i guess it still needs release notes?
< wumpus>
doc/release-notes.md currently even speaks of 0.19.1
< luke-jr>
wumpus: well, that's the idea - to try to get closer to the latter by asking users to upgrade as if it weree
< wumpus>
I can update that, and the authors/changelog, but if dunno anyone wants to write anything else