[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #29206 [Circumvention/Snowflake]: New design for client -- server protocol for Snowflake
#29206: New design for client -- server protocol for Snowflake
-------------------------------------------------+-------------------------
Reporter: cohosh | Owner: cohosh
Type: task | Status:
| needs_revision
Priority: Medium | Milestone:
Component: Circumvention/Snowflake | Version:
Severity: Normal | Resolution:
Keywords: ex-sponsor-19, anti-censorship- | Actual Points:
roadmap |
Parent ID: | Points: 6
Reviewer: dcf | Sponsor:
| Sponsor28-must
-------------------------------------------------+-------------------------
Comment (by cohosh):
Replying to [comment:17 dcf]:
> Replying to [comment:13 cohosh]:
> > I'm going to ask for an incremental review on this one, mostly just to
get another pair of eyes on what I've done and my next steps before I sink
more time into going possibly the wrong direction:
https://github.com/cohosh/snowflake/compare/sequencing
>
Thanks for the feedback! I've addressed most of the feedback so far here:
https://github.com/cohosh/snowflake/tree/sequencing
> The fields would be better as `uint32`, because the size of `int`
[https://golang.org/ref/spec#Numeric_types may vary] (though I think you
handled it correctly by always specifying 32 bits in serialization
functions). `length` may be better as `uint16`; that's plenty for one call
to `Write` and it avoids the risk that an implementation will try to
buffer 4 GB in memory.
Fixed in
[https://github.com/cohosh/snowflake/commit/c102894bb903857061e7705e9525612eb9a99b4b
c102894]
>
> IMO `length` should be ''exclusive'' of the header size. Otherwise you
have to define what it means if `length` < 12. (I believe the current code
doesn't work in this case because `header.length - snowflakeHeaderLen`
underflows.) The packets sent by `sendAck` should have `length` = 0.
>
Fixed in
[https://github.com/cohosh/snowflake/commit/02bfa1b4611db39a99379eefd29d569563490a0a
02bfa1b]
> Big-endian is more usual for network protocols.
>
Fixed in
[https://github.com/cohosh/snowflake/commit/7566cbc9e3d289a901d717e2ed5bc10a335915d7
7566cb]
> I'm not sure about the implementation of `Read`:
> I would prefer to start with an implementation of `Read` that is
simpler, if less efficient. I am picturing something like this [...]
I like this implementation a lot more. Fixed in
[https://github.com/cohosh/snowflake/commit/b6fb987ba9aca02c6740231df6c9db60b68993a6
b6fb987] and
[https://github.com/cohosh/snowflake/commit/9c1b12f7c645e0a8a95199190d89b84ed677e3f8
9c1b12f]
I'm going to start on implementing fixed size send windows and cleaning up
the unit tests a bit.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/29206#comment:19>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs