Added you! Hope it works!
Hey @rhatto! Feel free to pass me your readthedocs username and I will add you as an admin to the project!
We did it! It has been moved: https://github.com/torproject/onionbalance
Got the inv. I can transfer it there. Are we sure it's not https://github.com/torproject/ that we want it at?
ACK. That works as well.
I did the transfer.
Hey @gaba! Long time no see. Hope all is well over there as well!
I tried to transfer the repo to torproject
but it tells me You don’t have the permission to create public repositories on torproject
.
In https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository, it says To transfer a repository that you own to an organization, you must have permission to create a repository in the target organization.
.
Perhaps I can transfer it to a personal account with access to torproject, and they can then transfer to torproject?
Hello all. For some additional context, people have been submitting easy PRs in https://github.com/asn-d6/onionbalance and I've been merging them (two so far). AFAIK, there is no way to disable PRs in github, so I can't stop them from coming. Also, this gitlab repo is not mirroring the GH repo, so the gitlab repo is behind right now.
Would you like me to transfer the onionbalance GH repo to someone else, or to the torproject organization?
What about this one: https://packages.debian.org/search?keywords=onionbalance ?
I'm a bit rusty on this, but I think the effort is checked both in the top-half protection of R * E > UINT32_MAX
and also in the bottom-half protection of equix_verify(C || N || E, S) != EQUIX_OK
.
This makes me wonder how often we expect the Client checks if R * E <= UINT32_MAX event to trigger. The answer here impacts both how easily legitimate clients create valid tickets, and also how much top-half protection this blake2b feature adds in the first place.
This makes me think that the proposal is underspecified there, since R
is the output of blake2b (like 32 bytes) but then it's multipled by an unsigned integer and compared against UINT32_MAX. I think this doesn't make much sense.
Furthermore -- regardless of how the above computation is defined -- since E
is attacker controlled (but MUST be above MIN_EFFORT
), an attacker who wants to overwhelm the top half will set E==MIN_EFFORT
and then hope that blake2b will return a small enough number when given trash. How often this happens depends on the parameter tuning above.
Also, I this top-half protection has not been taken into account in the various parameter tuning sections of the proposal. If getting past the top-half protection is hard, the numbers will look different. If getting past the top-half protection is easy, then maybe it's not useful?
Feel free to ignore this nitpicking. I was hoping that if this blake2b top-half protection doesn't work that well, it might be removed from the proposal and hence make implementation slightly easier.
I think it might also be a good idea to re-examine tevador's intuition about this trick not being particularly effective. If that's the case, perhaps it shouldn't be implemented in the first place.
Hey @beth. Agreed, I think blake2b was chosen just because it's fast and probably because tevador felt comfortable with it. I think it's fine to swap it with any other fast hash function, to avoid adding an extra dependency.
OK I took some time for this ticket again:
a) The batch verification logic of donna is indeed problematic and the way that Tor does the fallback (see fallback
label when the batch verification check fails) in ed25519_checksig_batch()
can indeed cause crashes. The good thing is that the batch verification is never actually called (it's only enabled if four or more sigs are passed which never happens). Fixing the math would be a pretty big move since upstream is also broken, so I opted for removing the batching functionality. Using a proper library like ed25519-dalek in arti would allow us to do batch verification again. See Henry's excellent blog post for more details.
See !414 (closed) for the fix commit. I also pushed https://gitlab.torproject.org/asn/tor/-/tree/bug40078-test-vectors which makes sure that the remaining ZIP125 test vectors don't break tor. It's up to us whether we want to include the test vectors in tor. I saw that OpenSSL did not include them so I took the easy way out myself.
b) Henry's point (2) is also valid. We are indeed not doing the right check to see if the received S
is canonical. In particular, ed25519-donna does the weak check for S
canonicity (checking just the three most-significant bits) as described in the section Checking for non-canonical S
of this paper.
Our check will catch most non-canonical signatures but there are a bunch of them that can escape. In particular, the paper says that for honestly generated signatures, the probability that the fourth most significant bit (252th bit) is set is very small, roughly 1/2^{128} .
This means that an attacker can craft signatures that are malleable (for each such evil signature, there is another one that verifies). And also there is an extremely small chance that non-attacker signatures can be malleable. I'm not sure what are the implications of ed25519 signature malleability in our protocol. Certainly one can go around replay-caches; what other attacks are there?
Fixing this would require us to implement the full canonicity check as described by that paper. I didn't find any other C codebase that implements the full canonicity check (see Table 5
on the paper) so I opted to not try to do it at this point.
Fixes bug 40078.
As reported by hdevalence our batch verification logic can cause an assert crash.
The assert happens because when the batch verification of ed25519-donna fails,
the code in ed25519_checksig_batch()
falls back to doing a single
verification for each signature.
The crash occurs because batch verification failed, but then all signatures individually verified just fine.
That's because batch verification and single verification use a different equation which means that there are sigs that can pass single verification but fail batch verification.
Fixing this would require modding ed25519-donna which is not in scope for this ticket, and will be soon deprecated in favor of arti and ed25519-dalek, so my branch instead removes batch verification.
George Kadianakis (7fdbb193) at 30 Jul 13:56
Disable ed25519-donna's batch verification.
George Kadianakis (48cc4e4b) at 30 Jul 13:51
Disable ed25519-donna's batch verification.
... and 1547 more commits
In two months v2 is dead and the v2 code in OBv3 is really polluting the codebase. Killing it would be a great step to reducing OBv3 tech debt.