Ticket #7202: andrea_7199-7202_notes.txt

File andrea_7199-7202_notes.txt, 3.2 KB (added by andrea, 7 years ago)
Line 
1In 6bd72b711dd21d34e08fe2299dfe0bc5b4a72f41, you implement a linear-time data structure.  Do we expect this to be small or will you replace it later with something faster?
2 - Okay, this is just the one I commented on under 316f22fe141624d2b0a6ac6b25c4f4ece6d5e10f
3
4I'll assume the correctness of the curve25519 implementation in 8bd4d20a693dad9d3de743d8921d69762c50558e, since it's external and you've addressed portability issues in 9648ac43f1a1eb0ca200eed9177f41ee8324ca2a.
5
6In 316f22fe141624d2b0a6ac6b25c4f4ece6d5e10f:
7 - /* XXXX Does this possible early-return business threaten our security? */
8   - The first early return for the check if the node_id matches seems harmless, since the only thing it depends on is
9     something that's public anyway: the router's identity digest.
10
11   - Why do we need the map for keypair_bB, though?  Is the router's curve25519 public key not a once-per-router
12     public value?  If not, then this provides a timing-based test for whether a router has a particular key in
13     its map.  Does this leak anything dangerous?
14     - 0a685804f407e06dac8bbf338f8e38aaedcbc33f makes it seem like it has only ever one or two entries:
15       current and possibly previous keys - we need the map because the key is rotated and clients might not have
16       seen the new key yet.  Is this correct?  Since the keys then appear in the consensus, I think this can't
17       provide any information that wasn't already public.
18     - But in 18ff5cb448181f2090349d89a3f1bc1a02e54d26, the early-exit is removed.  Just being paranoid, or something
19       amiss in my reasoning above?
20
21 - Just as an aside making sure I understand the protocol properly, the essential thing here is that secret_input contains the X the server saw *and* has g^xy and g^xb in it, so by the latter it could only be computed by someone who knows x (as Y^x and B^x) or by someone who knows y and b (as X^y and X^b), so the client can check that the value of verify that it received was based on the same X that it sent and knows that it could only have been produced by the server?  We need the server ephemeral keypair (y,Y) because without it, compromising b lets an attacker compute secret_input for past recorded traffic and decrypt it?
22
23 - /* XXXXX VERIFY Y IS NOT POINT AT INFINITY */
24   - Fix this?
25   - Okay, af175dbe07d3fd712c8d7cf232d6715a55b8580d.  Is the all-zeros comparison the representation of the point at infinity here? (Sorry, I'm just reading up on elliptic curves as groups here; the point at infinity is always the group
26identity?)
27   - What was the 2007 DH bug mikeperry mentions in the comments on #7202?
28
29In 512ab64aa45e7cef83f1c54d7aeb3be69d22f48e;
30 - Generating the server ephemeral key means an attacker can force us to consume entropy.  What happens to the quality
31of the entropy we would be using for legitimate clients in that case?  Looks like crypto_strongest_rand() can't block to wait on real random bits, but I presume it's a strong enough PRNG that this isn't dangerous?
32
33I'm okay with all the create_cell_t, etc. refactoring assuming this has been tested thoroughly.
34
35This CREATE2-inside-CREATE business in 89e3dd8c3029e2319466fb47f33d31b76c870008 isn't in the proposal.  Is it going to get into the spec properly?