Noise: PatternXX and testing with go
#406
Conversation
|
Though there are things to be added and confirmed, I think this should be ready for review. We can talk with go implementation through |
| return int.from_bytes(length_bytes, byteorder=BYTE_ORDER) | ||
|
|
||
|
|
||
| def encode_msg_with_length(msg_bytes: bytes) -> bytes: | ||
| len_prefix = len(msg_bytes).to_bytes(SIZE_LEN_BYTES, "big") | ||
| def encode_msg_with_length(msg_bytes: bytes, size_len_bytes: int) -> bytes: |
There was a problem hiding this comment.
Why is the size passed in here instead of measuring it from len(msg_bytes)?
There was a problem hiding this comment.
size_len_bytes means the size of the length field in the message. In noise, it's 2 bytes. len(msg_bytes) is encoded and put in this length field, and therefore len(msg_bytes) should be < 2**(8*2) = 65536.
| read_closer: ReadCloser | ||
| next_length: Optional[int] | ||
| @abstractmethod | ||
| def encode_msg(self, msg: bytes) -> bytes: |
There was a problem hiding this comment.
should this be public? Is it used as an external API or only called internally?
There was a problem hiding this comment.
It's only called internally, but I expect subclasses to implement it to make write_msg work. So as next_msg_len, which is used internally but needs to be implemented by the subclasses to make read_msg work.
| read_closer: ReadCloser | ||
| next_length: Optional[int] | ||
| @abstractmethod | ||
| def encode_msg(self, msg: bytes) -> bytes: |
There was a problem hiding this comment.
It's only called internally, but I expect subclasses to implement it to make write_msg work. So as next_msg_len, which is used internally but needs to be implemented by the subclasses to make read_msg work.
| return int.from_bytes(length_bytes, byteorder=BYTE_ORDER) | ||
|
|
||
|
|
||
| def encode_msg_with_length(msg_bytes: bytes) -> bytes: | ||
| len_prefix = len(msg_bytes).to_bytes(SIZE_LEN_BYTES, "big") | ||
| def encode_msg_with_length(msg_bytes: bytes, size_len_bytes: int) -> bytes: |
There was a problem hiding this comment.
size_len_bytes means the size of the length field in the message. In noise, it's 2 bytes. len(msg_bytes) is encoded and put in this length field, and therefore len(msg_bytes) should be < 2**(8*2) = 65536.
TODO - Figure out why `state.rs` is erased at some moment(even handshake is not done). - Refactor - Add tests
TODO: Add a buffer to read only `n` bytes in `read(n)`
- Abstract it as `MsgReadWriter` - `MsgIOReadWriter` as a subclass of `MsgReadWriter`
Make security sessions(secio, noise) share the same implementation `BaseSession` to avoid duplicate implementation of buffered read.
`connection.py` is removed.
- Change `BaseMsgReadWriter` to encode/decode messages with abstract method, which can be implemented by the subclasses. This allows us to create subclasses `FixedSizeLenMsgReadWriter` and `VarIntLenMsgReadWriter`.
mhchia
force-pushed
the
feature/noise-patterxx
branch
from
f90fc46 to
ec8c109
Compare
|
I'm merging this PR since it gets too large and it already works fine with the go implementation. Will open a new one if everything gets updated. |
What was wrong?
#375
How was it fixed?
Based on #405
Pattern XXSecureSession(a buffered stream reader originally used in secio) is refactored and used by bothNoiseandSecionow.MsgReadWriterFixedSizeLenMsgReadWriterNoiseReadWriterVarIntLengthMsgReadWriterPlaintextHandshakeReadWriterVarIntLengthMsgReadWriteroverFixedSizeLenMsgReadWriter. Ref: use varints for delimiting plaintext 2.0 msgs go-libp2p-core#74go-libp2p-daemonagainst different security protocol.go-libp2p-noise32bytes for now. Will investigate what is wrong later.noise.protoplaintext.protoTODO
Reference
Cute Animal Picture